Simon Riggs <simon@2ndQuadrant.com> writes:
> Resetting it back seems fragile, since in crash recovery we call it
> again almost immediately during CreateCheckPoint(). That only works if
> LocalSetXLogInsertAllowed() has no side effects. I understand Heikki's
> wish to have safeguards in place, so we should document that
> LocalSetXLogInsertAllowed() can be executed twice without problem.
Done.
My initial thought had actually been to get rid of the use of
LocalSetXLogInsertAllowed inside CreateCheckPoint, since with this
patch we are calling it from the same bit of code that is about
to call CreateCheckPoint --- leaving the flag set throughout that
bit would be fine. However that would only work as intended if
the checkpoint were done locally; if somehow we'd launched the
bgwriter and the checkpoint request got sent over there, it'd fail.
I don't believe this is currently possible during a crash recovery
scenario, but on the whole it seemed more fragile to do it that way
than in the code as-committed. In principle, at least, it seems
possible that the rm_cleanup and checkpoint actions could be done
in different processes, and this setup preserves the freedom to
let that happen.
regards, tom lane