Re: pg_waldump error message fix

2020-12-14 Thread Bossart, Nathan
On 12/13/20, 7:01 PM, "Michael Paquier" wrote: > On Mon, Dec 14, 2020 at 10:26:01AM +0900, Kyotaro Horiguchi wrote: >> Yeah, I had the same feeling. At least, the two LSNs in the message >> under discussion are simply redundant. So +1 to just remove the LSN at >> the caller site. > > That would me

Re: pg_waldump error message fix

2020-12-13 Thread Michael Paquier
On Mon, Dec 14, 2020 at 11:34:51AM +0900, Kyotaro Horiguchi wrote: > Apart from this issue, while checking that, I noticed that if server > starts having WALs from a server of a different systemid, the server > stops with obscure messages. Wouldn't it be better to discuss that on a separate thread

Re: pg_waldump error message fix

2020-12-13 Thread Michael Paquier
On Mon, Dec 14, 2020 at 10:26:01AM +0900, Kyotaro Horiguchi wrote: > Yeah, I had the same feeling. At least, the two LSNs in the message > under discussion are simply redundant. So +1 to just remove the LSN at > the caller site. That would mean that we are ready to accept that we will never forget

Re: pg_waldump error message fix

2020-12-13 Thread Kyotaro Horiguchi
> At Fri, 11 Dec 2020 19:27:31 +, "Bossart, Nathan" > wrote in > > I looked through all the calls to report_invalid_record() in > > xlogreader.c and noticed that all but a few in > > XLogReaderValidatePageHeader() already report an LSN. Of the calls in > > XLogReaderValidatePageHeader() tha

Re: pg_waldump error message fix

2020-12-13 Thread Kyotaro Horiguchi
At Fri, 11 Dec 2020 19:27:31 +, "Bossart, Nathan" wrote in > On 12/10/20, 9:23 PM, "Michael Paquier" wrote: > > Please note that this is documented in xlogreader.h. Hmm. I can see > > your point here, still I think that what both of you are suggesting > > is not completely correct. For e

Re: pg_waldump error message fix

2020-12-11 Thread Bossart, Nathan
On 12/10/20, 9:23 PM, "Michael Paquier" wrote: > Please note that this is documented in xlogreader.h. Hmm. I can see > your point here, still I think that what both of you are suggesting > is not completely correct. For example, switching to EndRecPtr would > cause DecodeXLogRecord() to report

Re: pg_waldump error message fix

2020-12-11 Thread Kyotaro Horiguchi
At Fri, 11 Dec 2020 17:19:33 +0900 (JST), Kyotaro Horiguchi wrote in > At Fri, 11 Dec 2020 14:21:57 +0900, Michael Paquier > wrote in > > On Fri, Dec 11, 2020 at 01:30:16PM +0900, Kyotaro Horiguchi wrote: > > > currRecPtr is a private member of the struct (see the definition of > > > the stru

Re: pg_waldump error message fix

2020-12-11 Thread Kyotaro Horiguchi
At Fri, 11 Dec 2020 14:21:57 +0900, Michael Paquier wrote in > On Fri, Dec 11, 2020 at 01:30:16PM +0900, Kyotaro Horiguchi wrote: > > currRecPtr is a private member of the struct (see the definition of > > the struct XLogReaderState). We should instead use EndRecPtr outside > > xlog reader. >

Re: pg_waldump error message fix

2020-12-10 Thread Michael Paquier
On Fri, Dec 11, 2020 at 01:30:16PM +0900, Kyotaro Horiguchi wrote: > currRecPtr is a private member of the struct (see the definition of > the struct XLogReaderState). We should instead use EndRecPtr outside > xlog reader. Please note that this is documented in xlogreader.h. Hmm. I can see your

Re: pg_waldump error message fix

2020-12-10 Thread Kyotaro Horiguchi
At Thu, 10 Dec 2020 18:47:58 +, "Bossart, Nathan" wrote in > Hi, > > I noticed that when pg_waldump finds an invalid record, the > corresponding error message seems to point to the last valid record > read. Good catch! > rmgr: ... lsn: 0/090E5AF8, prev 0/090E59D0, ... > pg_waldump

pg_waldump error message fix

2020-12-10 Thread Bossart, Nathan
Hi, I noticed that when pg_waldump finds an invalid record, the corresponding error message seems to point to the last valid record read. rmgr: ... lsn: 0/090E5AF8, prev 0/090E59D0, ... pg_waldump: fatal: error in WAL record at 0/90E5AF8: invalid record length at 0/90E5B30: wanted 24, go