On Wed, May 26, 2021 at 10:17 PM Michael Paquier <michael@paquier.xyz> wrote:
- if (do_sync) + if (do_sync && total_files_modified) Here, I am on the edge. It could be an advantage to force a flush of the data folder anyway, no?
I was originally on the fence about including this as well, but it seems like since the database is shut down and already in a consistent state, there seems no advantage to syncing if we have not made any changes. Things are no better or worse than when we arrived. However, the real-world use case of running pg_checksums --enable and getting no changed blocks is probably fairly rare, so if there is a strong objection, I'm happy reverting to just (do_sync). (I'm not sure how cheap a sync is, I assume it's low impact as the database is shut down, I guess it becomes a "might as well while we are here"?)
In one of the checksum patches, there was an understanding that the pages should be written even if the checksum is correct, to handle replicas. ... Does your patch complicate things for the "stop all the clusters before switching them all" case?
I cannot imagine how it would, but, like Michael, I'm not really understanding the reasoning here. We only run when safely shutdown, so no WAL or dirty buffers need concern us :). Of course, once the postmaster is up and running, fiddling with checksums becomes vastly more complicated, as evidenced by that thread. I'm happy sticking to and speeding up the offline version for now.
