On Wed, Nov 16, 2016 at 7:09 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Nov 16, 2016 at 3:54 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Indeed I missed this comment block. Please let me suggest the following instead:
>> /*
>> * Set up an init fork for an unlogged table so that it can be correctly
>> - * reinitialized on restart. Since we're going to do an immediate sync, we
>> - * only need to xlog this if archiving or streaming is enabled. And the
>> - * immediate sync is required, because otherwise there's no guarantee that
>> - * this will hit the disk before the next checkpoint moves the redo pointer.
>> + * reinitialized on restart. An immediate sync is required even if the
>> + * page has been logged, because the write did not go through
>> + * shared_buffers and therefore a concurrent checkpoint may have moved
>> + * the redo pointer past our xlog record.
>> */
>
> Hmm. Well, that deletes the comment that's no longer true, but it
> doesn't replace it with any explanation of why we also need to WAL-log
> it unconditionally, and I think that explanation is not entirely
> trivial?
OK, the original code does not give any special reason either
regarding why doing so is safe for archiving or streaming :)
More seriously, if there could be more details regarding that, I would
think that we could say something like "logging the init fork is
mandatory in any case to ensure its on-disk presence when at recovery
replay, even on non-default tablespaces whose base location are
deleted and re-created from scratch if the WAL record in charge of
creating this tablespace gets replayed". The problem shows up because
of tablespaces being deleted at replay at the end... So perhaps this
makes sense. What do you think?
--
Michael