Trying to break online checksums with LLMs

On Tue, Sep 8, 2026 at 5:38 PM Daniel Gustafsson <daniel@yesql.se> wrote:

Noah kindly ran Claude- and GPT-automated reviews on the online checksums
feature with the fix from the open item applied. Since he has been very
successful in identifying previously unknown issues in other features I was
interested in seeing what could shake loose. The full reports are attached,
with an executive summary of the identified issues and the fixes below. It's
mostly documentation or corner cases from re-starting operations within tight
windows, none of the issues would lead to incorrect results or false negatives
or positives in page verification. Fixes are in separate patches for easier
review, for a commit I would group the doc ones together.

Nice.

Patch 0003's commit message writes "th" where "the" is intended.

Regarding 0005, Claude is suggesting to me that this might be the
wrong fix. It says (I think correctly) that the real problem is the
code at the end of launcher_exit, which is willing to set
DataChecksumState->launcher_running = false even if our process was
not the one that set that value to true. Unless our local copy of
launcher_running is true, we have no business calling
SetDataChecksumsOff() or setting DataChecksumState->launcher_running =
false. If you fix that, then perhaps it's OK for
DataChecksumsWorkerLauncherMain to stay as it is: the problem there is
that after DataChecksumsWorkerLauncherMain sets launcher_running back
to false, launcher_exit() can still run and decide to mess with shared
state.

Another small problem that Claude found: Within
DataChecksumsWorkerLauncherMain, if an abort is requested, we will set
abort_requested = true and goto done. From there we can goto again.
Now we're starting over, but abort_requested is still true, because
nothing resets it. If the user starts to enable checksums, then
changes their mind and starts to disable them, then changes their mind
again and starts to enable them, the second enable will get confused
and not complete properly. I think this is a one-line fix.

But much more seriously, Claude also noticed this: In general, if
PostgreSQL creates a relation and then crashes before commit, the
relation files are left on disk and not removed, but there's no
catalog entry pointing to them any more. If checksums are enabled
using pg_checksums, such files are updated to have checksums as well.
But this feature enables checksums by working through the catalog, so
it won't update those files. That means that if, at any point in the
history of the cluster, there's ever been a crash that left a relation
file behind on disk, and then you use this feature to enable
checksums, and then after that you take a base backup, checksum
verification will fail, and thus the backup will fail. I don't quite
see how we can actually fix this, and it seems like a serious problem.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:

But much more seriously, Claude also noticed this: In general, if
PostgreSQL creates a relation and then crashes before commit, the
relation files are left on disk and not removed, but there's no
catalog entry pointing to them any more. If checksums are enabled
using pg_checksums, such files are updated to have checksums as well.
But this feature enables checksums by working through the catalog, so
it won't update those files. That means that if, at any point in the
history of the cluster, there's ever been a crash that left a relation
file behind on disk, and then you use this feature to enable
checksums, and then after that you take a base backup, checksum
verification will fail, and thus the backup will fail. I don't quite
see how we can actually fix this, and it seems like a serious problem.

Don't we have logic somewhere to clean up orphaned relation files?

regards, tom lane

On Thu, Sep 10, 2026 at 4:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Don't we have logic somewhere to clean up orphaned relation files?

Uh ... no?

I mean, temp tables get cleaned up and unlogged tables get reset to
empty, but the goal of being able to remove permanent relation files
instead of permanently leaking them has, to my knowledge, thus far
eluded us. I had hope to address that when working on UNDO, and other
people have tried as well, but unless I've missed something big,
nobody has thus far succeeded.

--
Robert Haas
EDB: http://www.enterprisedb.com

On 9/10/26 22:37, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

But much more seriously, Claude also noticed this: In general, if
PostgreSQL creates a relation and then crashes before commit, the
relation files are left on disk and not removed, but there's no
catalog entry pointing to them any more. If checksums are enabled
using pg_checksums, such files are updated to have checksums as well.
But this feature enables checksums by working through the catalog, so
it won't update those files. That means that if, at any point in the
history of the cluster, there's ever been a crash that left a relation
file behind on disk, and then you use this feature to enable
checksums, and then after that you take a base backup, checksum
verification will fail, and thus the backup will fail. I don't quite
see how we can actually fix this, and it seems like a serious problem.

Don't we have logic somewhere to clean up orphaned relation files?

regards, tom lane

There is an item [1]https://wiki.postgresql.org/wiki/Todo#Administration on the TODO list, and a thread [2]/messages/by-id/200606081508.k58F85m29270@candle.pha.pa.us:

[1]: https://wiki.postgresql.org/wiki/Todo#Administration

[2]: /messages/by-id/200606081508.k58F85m29270@candle.pha.pa.us
/messages/by-id/200606081508.k58F85m29270@candle.pha.pa.us

On Thu, Sep 10, 2026, at 4:51 PM, Robert Haas wrote:

On Thu, Sep 10, 2026 at 4:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Don't we have logic somewhere to clean up orphaned relation files?

Uh ... no?

I mean, temp tables get cleaned up and unlogged tables get reset to
empty, but the goal of being able to remove permanent relation files
instead of permanently leaking them has, to my knowledge, thus far
eluded us. I had hope to address that when working on UNDO, and other
people have tried as well, but unless I've missed something big,
nobody has thus far succeeded.

This is incoming with my resurrection of the UNDO work you and other
did and the addition of what I call "FILEOPS".

ETA... a week, target v20.

-greg

On 10 Sep 2026, at 22:33, Robert Haas <robertmhaas@gmail.com> wrote:

Thanks for reviewing!

Patch 0003's commit message writes "th" where "the" is intended.

Ugh, will fix.

Regarding 0005, Claude is suggesting to me that this might be the
wrong fix. It says (I think correctly) that the real problem is the
code at the end of launcher_exit, which is willing to set
DataChecksumState->launcher_running = false even if our process was
not the one that set that value to true. Unless our local copy of
launcher_running is true, we have no business calling
SetDataChecksumsOff() or setting DataChecksumState->launcher_running =
false. If you fix that, then perhaps it's OK for
DataChecksumsWorkerLauncherMain to stay as it is: the problem there is
that after DataChecksumsWorkerLauncherMain sets launcher_running back
to false, launcher_exit() can still run and decide to mess with shared
state.

Interesting, I will have a look at that tomorrow morning after coffee.

Another small problem that Claude found: Within
DataChecksumsWorkerLauncherMain, if an abort is requested, we will set
abort_requested = true and goto done. From there we can goto again.
Now we're starting over, but abort_requested is still true, because
nothing resets it. If the user starts to enable checksums, then
changes their mind and starts to disable them, then changes their mind
again and starts to enable them, the second enable will get confused
and not complete properly. I think this is a one-line fix.

Right, in this case the second enable will not proceed and checksums will end
up disabled. Will have a look.

But much more seriously, Claude also noticed this: In general, if
PostgreSQL creates a relation and then crashes before commit, the
relation files are left on disk and not removed, but there's no
catalog entry pointing to them any more. If checksums are enabled
using pg_checksums, such files are updated to have checksums as well.
But this feature enables checksums by working through the catalog, so
it won't update those files. That means that if, at any point in the
history of the cluster, there's ever been a crash that left a relation
file behind on disk, and then you use this feature to enable
checksums, and then after that you take a base backup, checksum
verification will fail, and thus the backup will fail. I don't quite
see how we can actually fix this, and it seems like a serious problem.

I have a feeling this had been brought up in one of all the postcommit threads,
but I fail to find it now (perhaps it was a basebackup and not checksums
thread?). The gist of that discussion was that there is somewhere between very
little add nothing we can do to protect against orphaned/external files in the
datadir when using the catalog as the main source of truth for what we expect
it to contain. I wonder if the best we can do is to document exactly how to
verify the state of the page when getting a backup page verification failure?

--
Daniel Gustafsson

On Thu, Sep 10, 2026 at 5:08 PM Daniel Gustafsson <daniel@yesql.se> wrote:

I have a feeling this had been brought up in one of all the postcommit threads,
but I fail to find it now (perhaps it was a basebackup and not checksums
thread?). The gist of that discussion was that there is somewhere between very
little add nothing we can do to protect against orphaned/external files in the
datadir when using the catalog as the main source of truth for what we expect
it to contain. I wonder if the best we can do is to document exactly how to
verify the state of the page when getting a backup page verification failure?

I think that it is fine to disclaim external files -- if you add
random files to the data directory, stuff may break, and that's sad,
but oh well. But I think orphaned files are a completely different
ball game. That's just a normal outcome of running PostgreSQL for a
period of time under real-world conditions, and to my knowledge this
would be the first feature that doesn't work with them. I'd like to
hear what others think, but my first reaction is to think that's
probably not OK. If we don't remove the files automatically, and don't
even provide any tools to remove the files manually, I think it
follows logically that everything we ship has to cope with their
possible existence. Otherwise, it seems like we're shipping a feature
that sometimes doesn't work and for which we can't even provide
reliable steps to fix it when it doesn't.

--
Robert Haas

On 10 Sep 2026, at 23:35, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Sep 10, 2026 at 5:08 PM Daniel Gustafsson <daniel@yesql.se> wrote:

I have a feeling this had been brought up in one of all the postcommit threads,
but I fail to find it now (perhaps it was a basebackup and not checksums
thread?). The gist of that discussion was that there is somewhere between very
little add nothing we can do to protect against orphaned/external files in the
datadir when using the catalog as the main source of truth for what we expect
it to contain. I wonder if the best we can do is to document exactly how to
verify the state of the page when getting a backup page verification failure?

I think that it is fine to disclaim external files -- if you add
random files to the data directory, stuff may break, and that's sad,
but oh well. But I think orphaned files are a completely different
ball game. That's just a normal outcome of running PostgreSQL for a
period of time under real-world conditions, and to my knowledge this
would be the first feature that doesn't work with them. I'd like to
hear what others think, but my first reaction is to think that's
probably not OK. If we don't remove the files automatically, and don't
even provide any tools to remove the files manually, I think it
follows logically that everything we ship has to cope with their
possible existence. Otherwise, it seems like we're shipping a feature
that sometimes doesn't work and for which we can't even provide
reliable steps to fix it when it doesn't.

It is too close to midnight for investigation, so I am mostly asking an open
question as a TODO marker for myself in the morning: Are we guaranteed that a
create relation that crash will end up with orphan files containing correct
checksums? That is, if you have a cluster initdb'd with checksums on, and
never use neither online checksums or pg_checksums, can you end with an orphan
file from CREATE TABLE crashing which cause a backup page verification failure?

--
Daniel Gustafsson

On Thu, Sep 10, 2026 at 5:50 PM Daniel Gustafsson <daniel@yesql.se> wrote:

It is too close to midnight for investigation, so I am mostly asking an open
question as a TODO marker for myself in the morning: Are we guaranteed that a
create relation that crash will end up with orphan files containing correct
checksums? That is, if you have a cluster initdb'd with checksums on, and
never use neither online checksums or pg_checksums, can you end with an orphan
file from CREATE TABLE crashing which cause a backup page verification failure?

All of the blocks in an orphaned relation file were properly
WAL-logged when they were written, so they might be inconsistent at
the moment of the crash, but crash replay will fix them up so that
they have valid contents. From replay's point of view, those files
aren't orphaned at all, and it treats them just like any other
relation files. The problem is just that at a certain point they stop
being mentioned in the WAL stream any more, because the transaction
that caused them to be created gets terminated by the system going
down, and never has a chance to write additional WAL records asking
for those files to be removed, nor on the other hand to commit so that
the catalog rows that point to those files become visible.

Over time, as activity continues in the rest of the cluster but not in
the orphaned files, the xmin and xmax values in any tuples in those
orphaned files will drift away from what the rest of the cluster is
doing -- the *tuples* won't be readable any more -- but the *pages*
are fine. To base backups and anything else that operates at the
storage layer, they just look like pages we haven't chosen to write
for a long while.

--
Robert Haas

On Thu, Sep 10, 2026 at 06:00:31PM -0400, Robert Haas wrote:

On Thu, Sep 10, 2026 at 5:50 PM Daniel Gustafsson <daniel@yesql.se> wrote:

It is too close to midnight for investigation, so I am mostly asking an open
question as a TODO marker for myself in the morning: Are we guaranteed that a
create relation that crash will end up with orphan files containing correct
checksums? That is, if you have a cluster initdb'd with checksums on, and
never use neither online checksums or pg_checksums, can you end with an orphan
file from CREATE TABLE crashing which cause a backup page verification failure?

All of the blocks in an orphaned relation file were properly
WAL-logged when they were written, so they might be inconsistent at
the moment of the crash, but crash replay will fix them up so that
they have valid contents. From replay's point of view, those files
aren't orphaned at all, and it treats them just like any other
relation files. The problem is just that at a certain point they stop
being mentioned in the WAL stream any more, because the transaction
that caused them to be created gets terminated by the system going
down, and never has a chance to write additional WAL records asking
for those files to be removed, nor on the other hand to commit so that
the catalog rows that point to those files become visible.

Over time, as activity continues in the rest of the cluster but not in
the orphaned files, the xmin and xmax values in any tuples in those
orphaned files will drift away from what the rest of the cluster is
doing -- the *tuples* won't be readable any more -- but the *pages*
are fine. To base backups and anything else that operates at the
storage layer, they just look like pages we haven't chosen to write
for a long while.

Offhand, I expect checksum violations in orphan files can arise with
wal_level=minimal. If the transaction never reaches commit, or its commit
takes the smgrDoPendingSyncs() -> smgrdosyncall() path, the new relfilenode
has no WAL.

wal_level=minimal was last the default in v9.6, well before checksums were on
by default. Also, a checksum failure in pg_basebackup would require raising
wal_level after making the orphan. Those prerequisites may explain the lack
of reports.

On 10 Sep 2026, at 23:35, Robert Haas <robertmhaas@gmail.com> wrote:

I think that it is fine to disclaim external files -- if you add
random files to the data directory, stuff may break, and that's sad,
but oh well. But I think orphaned files are a completely different
ball game. That's just a normal outcome of running PostgreSQL for a
period of time under real-world conditions, and to my knowledge this
would be the first feature that doesn't work with them. I'd like to
hear what others think, but my first reaction is to think that's
probably not OK. If we don't remove the files automatically, and don't
even provide any tools to remove the files manually, I think it
follows logically that everything we ship has to cope with their
possible existence. Otherwise, it seems like we're shipping a feature
that sometimes doesn't work and for which we can't even provide
reliable steps to fix it when it doesn't.

Thinking about this over the weekend I think there is really only one possible
option. When enabling checksums online the data checksums launcher will have
to scan the datadir for orphan files, and abort processing if any are found
(and log which files it found). While we don't have a tool for clearning, I
don't think online checksums can, or should, handle the orphan files in any way
other than bailing out. This needs adding net new code, which this really
isn't a great time to be doing. CC:ing Heikki from RMT since I know he has
eyes on this already.

I ended up pushing the patchset for the other open item to master in the
meantime (after off-list +1 from RMT), but held off from backpatching to 19 at
the time of pushing till the BF has had a few builds of it. If we decide to
take online checksums out of 19 due to the above it's also less to back out.

--
Daniel Gustafsson

On Mon, Sep 14, 2026 at 9:47 AM Daniel Gustafsson <daniel@yesql.se> wrote:

Thinking about this over the weekend I think there is really only one possible
option. When enabling checksums online the data checksums launcher will have
to scan the datadir for orphan files, and abort processing if any are found
(and log which files it found). While we don't have a tool for clearning, I
don't think online checksums can, or should, handle the orphan files in any way
other than bailing out. This needs adding net new code, which this really
isn't a great time to be doing. CC:ing Heikki from RMT since I know he has
eyes on this already.

It's for the RMT to decide whether that's good enough, but I'm
skeptical. I think a big part of the problem here is that it's really
hard to make something race-free. But even if we solve that problem or
disclaim it, I feel like it doesn't leave users in a very good place.
A lot of users will experience failures and will have no easy way to
find their way out from under the problem. Against that, there's a
plausible argument that users want this feature enough that they'll
still be happier to have the feature despite that problem. My
counter-argument to that is that if we shipped features with usability
restrictions that severe in every release, I don't think we'd be where
we are today as a project. Limitations are fine, but limitations where
there's not much you can do to avoid the problem or recover from it
are pretty bad, IMHO.

--
Robert Haas

On 14 Sep 2026, at 21:59, Robert Haas <robertmhaas@gmail.com> wrote:

It's for the RMT to decide whether that's good enough, but I'm
skeptical. I think a big part of the problem here is that it's really
hard to make something race-free.

Once the cluster has moved to inprogress-on, any new orphan files will have
checksums written. This means that if the launcher can get a list of orphan
files once the state has changed, there cannot be any more created (without
checksums) during processing. There can still be new orphan files *with*
checksums written, but those would be no different from what we have today with
pg_checksums processing.

This reduction in problem scope likely means we can avoid the race conditions
that such scans are usually subject to. This is back-of-napkin sketching but I
think this invariant of the orphan files problem is less hard than the generic
one.

But at any rate, this is clearly not the time to design that.

If we revert we should keep online checksums in master and only pull it out of
19, to allow for this to be fixed early in 20, and avoid the churn. I held off
from backpatching the open item fix for this very reason; unless I hear
otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
Adding RMT to the thread to keep everyone in the loop.

--
Daniel Gustafsson

On Mon, Sep 14, 2026 at 5:59 PM Daniel Gustafsson <daniel@yesql.se> wrote:

Once the cluster has moved to inprogress-on, any new orphan files will have
checksums written. This means that if the launcher can get a list of orphan
files once the state has changed, there cannot be any more created (without
checksums) during processing. There can still be new orphan files *with*
checksums written, but those would be no different from what we have today with
pg_checksums processing.

That's a very good point. One thing that bothers me is: why don't we
just process the files by directory scan to begin with, instead of
trying to get at them via pg_class and then using this mechanism to
mop up the leftovers? I suspect the answer is locking. For an actually
orphaned file, we don't need to lock anything. But it might be hard to
tell the difference between a file that corresponds to nothing we saw
in pg_class because it's orphaned and a file that corresponds to
nothing we saw in pg_class because it was concurrently created and the
pg_class entry isn't yet visible to us. And, in the latter case,
skipping locking would probably be bad for all the same reasons it
would be bad to do that in general. But if you can find some way to
distinguish those cases reliably, then I think this might be a robust
fix.

--
Robert Haas

Hi,

On Tue, Sep 15, 2026 at 1:31 AM Robert Haas <robertmhaas@gmail.com> wrote:

But it might be hard to
tell the difference between a file that corresponds to nothing we saw
in pg_class because it's orphaned and a file that corresponds to
nothing we saw in pg_class because it was concurrently created and the
pg_class entry isn't yet visible to us. And, in the latter case,
skipping locking would probably be bad for all the same reasons it
would be bad to do that in general. But if you can find some way to
distinguish those cases reliably, then I think this might be a robust
fix.

pg_orphaned ([1]https://github.com/bdrouvot/pg_orphaned) is using a dirty snapshot while looking for the
relfilnode(s) in
pg_class to distinguish those cases. Maybe we could do the same here?

[1]: https://github.com/bdrouvot/pg_orphaned

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Mon, Sep 14, 2026 at 11:59:15PM +0200, Daniel Gustafsson wrote:

But at any rate, this is clearly not the time to design that.

If we revert we should keep online checksums in master and only pull it out of
19, to allow for this to be fixed early in 20, and avoid the churn. I held off
from backpatching the open item fix for this very reason; unless I hear
otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
Adding RMT to the thread to keep everyone in the loop.

Thanks for the heads up. Based on the discussion, it sounds like reverting
is the right call, unfortunately.

--
nathan

On 15 Sep 2026, at 13:59, Nathan Bossart <nathandbossart@gmail.com> wrote:

On Mon, Sep 14, 2026 at 11:59:15PM +0200, Daniel Gustafsson wrote:

But at any rate, this is clearly not the time to design that.

If we revert we should keep online checksums in master and only pull it out of
19, to allow for this to be fixed early in 20, and avoid the churn. I held off
from backpatching the open item fix for this very reason; unless I hear
otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
Adding RMT to the thread to keep everyone in the loop.

Thanks for the heads up. Based on the discussion, it sounds like reverting
is the right call, unfortunately.

I target doing it early tomorrow my time to allow lots of buildfarm monitoring
time, and to have time to look over the patch tonight.

--
Daniel Gustafsson

On 15 Sep 2026, at 14:18, Daniel Gustafsson <daniel@yesql.se> wrote:

On 15 Sep 2026, at 13:59, Nathan Bossart <nathandbossart@gmail.com> wrote:

Thanks for the heads up. Based on the discussion, it sounds like reverting
is the right call, unfortunately.

I target doing it early tomorrow my time to allow lots of buildfarm monitoring
time, and to have time to look over the patch tonight.

This was done earlier today.

--
Daniel Gustafsson