Heikki Linnakangas <hlinnakangas@vmware.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.
        regards, tom lane