Re: Only first XLogRecData is visible to rm_desc with WAL_DEBUG

On Mon, Mar 24, 2014 at 7:14 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
>> As we all know, when you compile with WAL_DEBUG, and enable wal_debug
>> GUC, you get output like this in the log for every inserted WAL record:
>
>> LOG: INSERT @ 0/5407E578: prev 0/5407E4D0; xid 0; len 32: Standby -
>> running xacts: nextXid 774 latestCompletedXid 771 oldestRunningXid 772;
>> 2 xacts: 7877929 0
>
>> What I did *not* realize before is that the redo routine only gets
>> passed the first XLogRecData struct in the chain. Because of that, the
>> "xacts: 7877929 0" information above is garbage. That's probably not the
>> only rm_desc routine that didn't get the memo.
>
> Yeah, I recall having fixed at least one such bug in the past.
>
>> There are a few alternatives on how to fix that:
>
>> 1. Do nothing. Add a comment somewhere explaining that rm_redo cannot
>> safely look at data beyond what's inserted in the first XLogRecData.
>
> We'll probably just make the same mistake again :-(. And I think there
> already is such a comment someplace.
>
>> 2. Reconstruct the WAL data from the XLogRecData entries in a palloc'd
>> buffer, and pass that to rm_redo. That would be fairly expensive, but
>> you probably don't care about that if you've enabled wal_debug.
>
>> 3. Remove the feature altogether, so that enabling wal_debug doesn't
>> cause all insertions to be logged anymore (no changes to the logging
>> during replay). It's a lot less interesting now that we have pg_xlogdump.
>
> I think the main use-case for rm_desc anymore is making CONTEXT lines
> for errors reported during WAL replay. I guess that situation does not
> have the same problem, since we've already loaded the complete WAL record.
>
> However, I'm not sure how easy it's going to be for WAL_DEBUG to make the
> data look the same as the replay case: in particular, substitution of
> full-page-images for data would be tough to predict in advance (and moving
> the printout into the critical section seems like a bad answer).
>
> I'd be okay with removing WAL_DEBUG, I think, particularly in view of the
> fact that there have been no requests to make it a compiled-by-default
> feature.

I've found WAL_DEBUG quite useful in the past, when working on
scalability, and have indeed wished for it to be
compiled-in-by-default.

I don't know whether I'm the only one, though.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company