pg_control read error message
If pg_control is for some reason empty, we give an error messagfe like:
2018-05-18 13:24:03.342 CEST [19697] PANIC: could not read from control
file: Success
Which is, uh, wrong -- it's definitely not successful.
Obviously this is a state where the user is fairly screwed anyway, but we
should give a better message.
Attached is a patch that does this. Reasonable?
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Attachments:
pg_control_error.patchtext/x-patch; charset=US-ASCII; name=pg_control_error.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 04bfac7485..adbd6a2126 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4486,6 +4486,7 @@ ReadControlFile(void)
pg_crc32c crc;
int fd;
static char wal_segsz_str[20];
+ int r;
/*
* Read data...
@@ -4499,10 +4500,17 @@ ReadControlFile(void)
XLOG_CONTROL_FILE)));
pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_READ);
- if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))
- ereport(PANIC,
- (errcode_for_file_access(),
- errmsg("could not read from control file: %m")));
+ r = read(fd, ControlFile, sizeof(ControlFileData));
+ if (r != sizeof(ControlFileData))
+ {
+ if (r < 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not read from control file: %m")));
+ else
+ ereport(PANIC,
+ (errmsg("could not read from control file: read %d bytes, expected %d", r, (int) sizeof(ControlFileData))));
+ }
pgstat_report_wait_end();
close(fd);
On 18/05/18 14:32, Magnus Hagander wrote:
If pg_control is for some reason empty, we give an error messagfe like:
2018-05-18 13:24:03.342 CEST [19697] PANIC: could not read from control
file: SuccessWhich is, uh, wrong -- it's definitely not successful.
Obviously this is a state where the user is fairly screwed anyway, but we
should give a better message.Attached is a patch that does this. Reasonable?
Looks good to me.
- Heikki
On Fri, May 18, 2018 at 03:54:47PM +0300, Heikki Linnakangas wrote:
Looks good to me.
+1 for fixing that. get_controlfile() in controldata_utils.c also needs
to be fixed.
--
Michael
Heikki Linnakangas <hlinnaka@iki.fi> writes:
On 18/05/18 14:32, Magnus Hagander wrote:
If pg_control is for some reason empty, we give an error messagfe like:
2018-05-18 13:24:03.342 CEST [19697] PANIC: could not read from control
file: Success
Which is, uh, wrong -- it's definitely not successful.
Obviously this is a state where the user is fairly screwed anyway, but we
should give a better message.
Attached is a patch that does this. Reasonable?
Looks good to me.
Only comment I have is that I think there's similar shortcuts in a lot
of places :-(
regards, tom lane
On Fri, May 18, 2018 at 4:02 PM, Michael Paquier <michael@paquier.xyz>
wrote:
On Fri, May 18, 2018 at 03:54:47PM +0300, Heikki Linnakangas wrote:
Looks good to me.
+1 for fixing that. get_controlfile() in controldata_utils.c also needs
to be fixed.
Pushed a fix including the controldata_utils.c one.
Per Toms comments we probably have more of these, but that's no excuse not
to fix this one :)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On Fri, May 18, 2018 at 10:22:29AM -0400, Tom Lane wrote:
Only comment I have is that I think there's similar shortcuts in a lot
of places :-(
Yeah. A quick lookup is showing me one in xlog.c (XLOG_BLCKSZ) and one
in pg_rewind. (Spotted roughly 392 places to look at in all the core
code). Let's discuss that on a separate thread.
--
Michael