Re: Offline data checksum changes can cause incorrect checksum state on standbys

> On 3 Sep 2026, at 13:54, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> On Thu, Sep 03, 2026 at 12:06:42PM +0100, Zsolt Parragi wrote:

>> v12 addresses these, otherwise it is unchanged to compared 11.
>
> Thanks! v12 LGTM.

Thanks for review. I've attached a v13 where I've moved most of the new tests
under PG_TEST_EXTRA to keep test times down. I placed most tests under
'checksum' and 18, 21 and 23 under 'checksum_extended', but the exact split may
be tweaked further. Since the origin of this open item is missing test
coverage, I prefer to add all these tests even though they aren't executed
during normal testruns. There are at least one BF animal running the full
suite which ensures timely execution of the tests.

This concludes the only open item left (thus far). Being able to error
standbys out of mismatched clusters would be nice, and is a potential
development area for 20, but it's not a showstopper if we never add it IMHO.

My current plan is to commit this to master only either tomorrow or Monday
after staring at it a little bit more, to a) give it exposure in the buildfarm
before an eventual backpatching; b) allow time for the revert discussion. If
we decide to revert I prefer to avoid more v19 churn.

On the latter topic. Since this thread is about closing the final open item
with the deadline for a new beta looming, it seems like a good place to bring
up the discussion of reverting. I am admittedly not sure about what the best
course of action would be, and some input is greatly appreciated. AFAICT these
are the main reasons for a revert:

* Architectural concerns
* Existing unfixed bugs (open items)
* Forwards incompatible post feature-freeze fixes of stopcap nature
* Potential bugs we don't know about
* RMT mandating a revert

I'll try to address each below, apart from the latter one.

Re-reading the recent threads, I don't see any reports of architectural
concerns which can be acted on. Pointers to threads would be appreciated If
I've missed any.

There are no further open issues (after this one). The argument for not
applying this patchset and closing the item would be that it's too complicated
and invasive at this point in the cycle (requires a pg_control change for
example). I have a lot of sympathy for this. The 0001 diffstat might seem
terrifying due to the new tests, but there is also a nontrivial amount of code
added to xlog.c. 0002 and onwards are addressing existing bugs in offline
checksums, though they will look a tad different if online checksums is
reverted.

The number of postcommit fixes was highlighted as one area of concern. Since
number of commits is a poor measurement of anything I've tried to quantify it
by compiling all the postcommit fixes and categorized them (see attached .txt).
Of the 29 thus far there have been 12 bugfixes, 14 spelling/typo fixes, source
code improvements and test stabilization. 3 commits are improvements or
optimizations. Whether or not those numbers are more interesting than 29, or
if either makes a case for reverting, I don't know. What I do know from
looking them over is that none of them were solved any different than what they
would have been if they were only in master.

Another case for a revert is if there is a general unspecified uneasiness over
the feature or its readiness among the maintainers and/or the RMT. And that's
totally fair. It's by far the most complicated and ambitious thing I've ever
built for Postgres. I have no interest in causing stress or worry in my fellow
postgres hackers, especially now, so if there is concensus on bad gut-feeling
then that's also a fair case for reverting. However, if we decide to revert
over an unspecified reason, I think we need to couple that with defining what
needs to change/happen over the v20 cycle to put that uneasiness to rest.
Simply leaving it in master without actionable technical concerns raised
against the code or the architecture for a year won't change anything.

Once/if this patchset lands, left on my TODO is to look at potentially dead
code around setting delayChkptFlags which Tomas Vondra identified, and keep
trying to break the code with the help of various LLM models and their ability
to see gaps in testing. I have posted a revert commit (to the original thread)
to give anyone interested a chance to see what it would look like, and what I
propose leaving behind in v19.

Any thoughts/concerns?

--
Daniel Gustafsson