I'd not do many of these at once. Do one simple conversion, with the relevant
analysis, and see whether it goes anywhere and whether you need to adjust your
approach. After that maybe 2 at once.
I don't think one commit per block is quite right either. You'll typically
have to add comments to the read and write side of some specific state
together, splitting that into separate commits doesn't make sense.
And the code needs to document the rules, not just the commit message.
> The analysis will be way longer than the changes themselves.
Of course.
Trying the checkpointer, I don't know much about it as a whole, I identified
the logic from the code, combined flags and a version number in a
pg_atomic_u32 state.
the { .ckpt_failed++; .ckpt_done = .ckpt_started } update
handled by inserting a memory barrier, to ensure .ckpt_failed is properly sampled.
the {.ckpt_flags = 0, .ckpt_started++} was handled by detecting the race afterwards
read .ckpt_started before and after setting the flags, if it changed, check the state's
version to determine whether it started before or after RequestCheckpoint setted
its flags.
Identified something unexpected (to me): RequestCheckpoint can fail due to a
failure on a checkpoint started before checkpointer seeing the updated flags.
Added a slightly different error message for that condition, just in case.
I imaging that this will not impact performance at all, but I hope it helps us
Regards,
Alexandre.
