>> Why don't we just use a barrier when around reading the value? It's not like >> CreateCheckPoint() is frequent?
One reason is that a barrier isn’t needed, and adding unnecessary barriers can also be confusing.
With respect to the “debug only” comment in the original code, whichever value is written to the structure during a checkpoint will be reset when restarting. Nobody will ever see that value. We could just as easily write a zero.
Shutting down is a different story. It isn’t stated, but we require the unlogged LSN be quiescent before shutting down. This patch doesn’t change that requirement.
We could also argue that memory ordering doesn’t matter when filling in a conventional, unlocked structure. But the fact we have only two cases 1) checkpoint where the value is ignored, and 2) shutdown where it is quiescent, makes the additional argument unnecessary.
Would you be more comfortable if I added comments describing the two cases? My intent was to be minimalistic, but the original code could use better explanation.
