On 19.05.2013 17:25, Simon Riggs wrote:
However, there is a call to RecoveryInProgress() at the top of the
main loop of the checkpointer, which does explicitly state that it
"initializes TimeLineID if it's not set yet." The checkpointer makes
the decision about whether to run a restartpoint or a checkpoint
directly from the answer to that, modified only in the case of an
CHECKPOINT_END_OF_RECOVERY.

So the appropiate call is made and I don't agree with the statement
that this "doesn't get executed with fast promotion", or the title of
the thread.

Hmm, I see.

So while I believe that the checkpointer might have an incorrect TLI
and that you've seen a bug, what isn't clear is that the checkpointer
is the only process that would see an incorrect TLI, or why such
processes see an incorrect TLI. It seems equally likely at this point
that the TLI may be set incorrectly somehow and that is why it is
being read incorrectly.

Yeah. I spent some more time debugging this, and found the culprit. In CreateRestartPoint, we do this:

                /*
                 * Update ThisTimeLineID to the timeline we're currently 
replaying,
                 * so that we install any recycled segments on that timeline.
                 *
                 * There is no guarantee that the WAL segments will be useful 
on the
                 * current timeline; if recovery proceeds to a new timeline 
right
                 * after this, the pre-allocated WAL segments on this timeline 
will
                 * not be used, and will go wasted until recycled on the next
                 * restartpoint. We'll live with that.
                 */
                (void) GetXLogReplayRecPtr(&ThisTimeLineID);

Here's what happens:

0. System is in recovery
1. checkpointer process starts creating a restartpoint.
2. Startup process ends recovery, creates end-of-recovery record, sets the shared ThisTimeLineID value to 2, and exits. 3. checkpointer process calls RecoveryInProgess() while performing the restartpoint (there are RecoveryInProgress() calls in many places, e.g in XLogNeedsFlush()). It sets LocalRecoveryInProgress = true, and initializes ThisTimeLineID to 2. 4. At the end of restartpoint, the checkpointer process does the above GetXLogReplayRecPtr(&ThisTimeLineID). That overwrites ThisTimeLineID back to 1. 5. Checkpointer calls RecoveryInProgress, which returns true immediately, as LocalRecoveryInProgress is already set. 5. Checkpointer performs the first checkpoint, with ThisTimeLineID set incorrectly to 1.

Not sure what the best fix would be. Perhaps change the code in CreateRestartPoint() to do something like this instead:

GetXLogReplayRecPtr(&replayTLI);
if (RecoveryInProgress())
  ThisTimeLineID = replayTLI;


I see that the comment in InitXLOGAccess() is incorrect
"ThisTimeLineID doesn't change so we need no lock to copy it", which
seems worth correcting since there's no need to save time in a once
only function.

Hmm, the code seems right to me, XLogCtl->ThisTimeLineID indeed doesn't change, once it's set. And InitXLOGAccess() isn't called until it is set. The comment could explain that better, though.

- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to