| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Trying to break online checksums with LLMs |
| Date: | 2026-09-08 21:38:19 |
| Message-ID: | E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Claude:
-------
* D1: Cancelling the enable launcher lets a second launcher start; the race
logs a false "data checksums are now enabled" while the cluster is off, and the
enable is silently lost
- This can cause an incorrect log entry, and the user will have to restart once
the exit handler is done which is surprising. The fix is to keep the
launcher_running shmem flag set to true during the exit handler as well. Fixed
on 0005.
* D2: Killing the launcher mid-*disable* leaves the cluster in `inprogress-off`
with nothing to finish the transition (an interrupted *enable* self-heals; a
disable does not)
- This preserves the state at inprogress-off and require a new disable
operation to clear it to off. The fix would be to execute
SetDataChecksumsOff() in the exit handler for this state as well, which I opted
out of doing since if you're terminating due to SetDataChecksumsOff() is
misbehaving, re-running it immediately again in the exit handlar is unlikely
what you want. Leaving it like this allows for using an offline disable in the
(perhaps unlikely) situation that SetDataChecksumsOff is broken.
* D3: pg_upgrade tells the user checksums are "being enabled" when the old
cluster was interrupted mid-*disable*
- Both inprogress-on and inprogress-off used the same pg_fatal error message
which indeed is misleading. Fixed in 0004.
* D4: pg_settings.enumvals for data_checksums is the phantom {""} instead of
the value list or NULL
- This is correct, the enum struct had the visibility flag backwards. Fixed in
0001.
* D5: pg_stat_progress_data_checksums.blocks_total/blocks_done are per-fork
though documented as per-relation, so blocks_done moves backward within one
relation
- Another correct finding, the documentation said "relation" but it should say
"relation fork", fixed in 0003. A future improvement would be to make the
current fork visible in the progress reporting (or switch to per-relation).
* D6: pg_enable/disable_data_checksums ship the delegable {POSTGRES=X} ACL yet
a hardcoded superuser() gate makes a granted EXECUTE silently ineffective; the
docs state no privilege model
- Correct, the ACL implied that access could be granted, but the functions are
strictly superuser only. Fixed, and the documentation updated in 0002.
GPT:
----
* A late temporary table is treated as pre-existing: Online enablement waits
for a session that it need not wait for
- The fact that processing need to wait for temporary tables is documented, and
while doing so it will use a specialized wait event highlighting this in the
progress reporting. It's true that the code can be optimized to not wait for
temporary tables which were created after the state changed to inprogress-on
but that's an improvement for future versions.
* pg_settings.enumvals` is {""}: Monitoring and introspection clients receive
false metadata
- Duplicate of Claude D4, fixed.
* A registered worker can outlive a terminated launcher: Relation I/O and
full-page-image WAL can begin after `SHOW data_checksums` reports `off` and the
launcher has exited
- This requires termination within a very tight window, and cannot lead to
wrong results or corruption. Will look at a fix.
* pg_upgrade misreports inprogress-off: Its fatal diagnostic says checksums are
being enabled while they are being disabled
- Duplicate of Claude D3, fixed.
* Progress block counters are per fork, not per relation as documented:
Monitors see unexplained resets and totals
- Duplicate of Claude D5, fixed.
* The documented function signature omits a valid one-argument call: Users are
not told they can set only cost_delay
- Fixed in 0003
* The function documentation omits the mandatory superuser requirement: Users
cannot tell who is authorized to run the operations
- Partial duplicate of Claude D6, fixed.
--
Daniel Gustafsson
| Attachment | Content-Type | Size |
|---|---|---|
| f19c0ec-checksum-online-claude-v0.patch | application/octet-stream | 81.7 KB |
| f19c0ec-checksum-online-gpt-v0.patch | application/octet-stream | 31.7 KB |
| v1-0001-Fix-enum-value-visibility-for-data_checksums.patch | application/octet-stream | 1.6 KB |
| v1-0002-doc-Add-missing-superuser-note-on-online-checksum.patch | application/octet-stream | 2.9 KB |
| v1-0003-doc-Documentation-fixups-for-online-checksums.patch | application/octet-stream | 3.3 KB |
| v1-0005-Only-reset-launcher-state-in-exit-handler.patch | application/octet-stream | 1.2 KB |
| v1-0004-Improve-error-message-for-checksum-state-in-pg_up.patch | application/octet-stream | 1.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-09-08 21:42:16 | Improve WAIT FOR read-your-writes consistency doc |
| Previous Message | Zsolt Parragi | 2026-09-08 21:21:42 | Re: Offline data checksum changes can cause incorrect checksum state on standbys |