Hello,
> > - xlog.c: Make StandbyMode shared.
> >
> > - checkpointer.c: Use IsStandbyMode() to check if postmaster is
> > under standby mode.
>
> IsStandbyMode() looks overkill to me. The standby mode flag is forcibly
> turned off at the end of recovery, but its change doesn't need to be shared
> to the checkpointer process, IOW, the shared flag doesn't need to change
> since startup like XLogCtl->archiveCleanupCommand, I think. So we can
> simplify the code to share the flag to the checkpointer. See the attached
> patch (though not tested yet).
Hmm. I understood that the aim of the spinlock and volatil'ize of
the pointer in reading shared memory is to secure the memory
consistency on SMPs with weak memory consistency and to make
compiler help from over-optimization for non-volatile pointer
respectively.
You removed both of them in the patch.
If we are allowed to be tolerant of the temporary lack of
coherence in shared memory there, the spinlock could be removed.
But the possibility to read garbage by using XLogCtl itself to
access standbyMode does not seem to be tolerable. What do you
think about that?
> The comments in checkpointer.c seems to need to be revised more. For
> example,
>
> + * XLogInsert that actually triggers a checkpoint when
>
> Currently a checkpoint is triggered by XLogWrite (not XLogInsert), the above
> needs to be corrected.
I will be carefull for such outdated description.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
== My e-mail address has been changed since Apr. 1, 2012.
