On Wednesday, September 12, 2012 5:33 AM Gurjeet Singh wrote:
> This comment in UpdateFullPageWrites() seems to be inaccurate:
> * It's safe to check the shared full_page_writes without the lock, > * because we assume that there is no concurrently running process which > * can update it.
>That assumption does not hold on any sane SMP system.
Do you able to see any case where it can be updated when being accessed here.
Now that I looked again, I don't see this being called by anyone other than Checkpointer or the Startup process (StartupXLOG()).
This stack seemed like it could be called by multiple backend processes at the same time:
But looking closely, CheckpointWriteDelay() has this check at the beginning:
if (!AmCheckpointerProcess()) return;
which stops normal backends from calling UpdateFullPageWrites(). All this is not obvious from the comments in UpdateFullPageWrites(), but the comments for XLogCtlInsert.fullPageWrites make it clear that this shared variable is changed only by the Checkpointer.
Thinking a bit more about the need for locks, I guess even the shared variables whose read/write ops are considered atomic need to be protected by locks so that the effects of NUMA architectures can be mitigated.
