pgsql: Revert online data checksum transitions

[pg-hub-mirror[bot]]

Read-only mirror. Join the conversation on pgsql-hackers; activity here is not sent upstream.

• Original author: Daniel Gustafsson <dgustafsson(at)postgresql(dot)org>
• Mailing list: pgsql-hackers
• Message-ID: E1x6lbC-00000000Yw0-3JVv@gemulon.postgresql.org
• Original email

---

Revert online data checksum transitions
The feature to enable, or disable, data checksums in an online
cluster saw a number of postcommit fixes during the beta period.
Suspicions were raised about the risk of more issues surfacing
after GA. To avoid shipping code which may have bugs, this
reverts in full, or in part, the following commits:
aaf8b99, cd857de, 5fee7ca, 25b922e, 8fb8ded,
b3a37ff, abac86c, 3a18526, 01805b7, 343d98c,
e469e47, 8d22f52, e5e1f6d, 5ab239c, 0ca1b30,
9a39056, 2018bd6, 1df361e, bf25e55, 381d19d,
b120358, 0700912, d771b0a, f19c0ec, 0907112,
397f0fd, 602f19c, e3a27ca, 4ae3e98, b364828
A few bits remain as they have merit on their own:

• The fix for offline checksums not preserving initial state in
  pg_control_init. The test case for this fix has been moved to
  src/bin/pg_checksums/t/002_actions.pl to survive the revert
  which removed the previous location.
• When a page verification is logged, the additional note which
  informs if the buffer will be zeroed out was kept to aid any
  debugging around checksum failures.
• The Data Checksum section in the docs was kept, but rewritten
  to reflect the current state.
• The enum containing checksum states is kept and some hardcoded
  references to the off state (0) are replaced with the label
  PG_DATA_CHECKSUM_OFF instead.
  Discussion: https://postgr.es/m/E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se
  Discussion: https://postgr.es/m/anwm6UPxoVS41QA2@bdtpg
  Discussion: https://postgr.es/m/CA+Tgmob9NY6m0YNFTQ4nFH2d0iC9SQRruDYxfndGKKzh8OC80w@mail.gmail.com
  Discussion: https://postgr.es/m/E07A611B-9CF3-4FDB-8CE8-A221E39040EC%40yesql.se
  Branch

---

REL_19_STABLE
Details

https://git.postgresql.org/pg/commitdiff/c05d5ce12366f1008e95b09a0a9b08a49818b0e1
Modified Files

doc/src/sgml/config.sgml | 5 +-
doc/src/sgml/func/func-admin.sgml | 79 -
doc/src/sgml/func/func-info.sgml | 5 -
doc/src/sgml/glossary.sgml | 20 +-
doc/src/sgml/images/Makefile | 1 -
doc/src/sgml/images/datachecksums.gv | 14 -
doc/src/sgml/images/datachecksums.svg | 81 -
doc/src/sgml/monitoring.sgml | 228 +--
doc/src/sgml/ref/pg_basebackup.sgml | 6 -
doc/src/sgml/ref/pg_checksums.sgml | 12 +-
doc/src/sgml/regress.sgml | 14 -
doc/src/sgml/release-19.sgml | 35 -
doc/src/sgml/wal.sgml | 185 +-
src/backend/access/rmgrdesc/xlogdesc.c | 58 +-
src/backend/access/transam/xlog.c | 552 +-----
src/backend/backup/basebackup.c | 90 +-
src/backend/catalog/system_views.sql | 19 -
src/backend/commands/dbcommands.c | 34 -
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/auxprocess.c | 28 -
src/backend/postmaster/bgworker.c | 9 -
src/backend/postmaster/datachecksum_state.c | 1979 --------------------
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/postmaster.c | 5 -
src/backend/replication/logical/decode.c | 16 -
src/backend/storage/ipc/procsignal.c | 8 -
src/backend/storage/page/README | 4 +-
src/backend/storage/page/bufpage.c | 18 +-
src/backend/utils/activity/pgstat_backend.c | 2 -
src/backend/utils/activity/pgstat_io.c | 35 -
src/backend/utils/activity/wait_event_names.txt | 3 -
src/backend/utils/adt/pgstatfuncs.c | 8 +-
src/backend/utils/init/miscinit.c | 3 +-
src/backend/utils/init/postinit.c | 30 +-
src/backend/utils/misc/guc_parameters.dat | 6 +-
src/backend/utils/misc/guc_tables.c | 9 +-
src/backend/utils/misc/pg_controldata.c | 9 +-
src/backend/utils/misc/postgresql.conf.sample | 10 +-
src/bin/pg_checksums/t/002_actions.pl | 5 +
src/bin/pg_controldata/pg_controldata.c | 2 -
src/bin/pg_upgrade/controldata.c | 9 -
src/bin/pg_waldump/t/001_basic.pl | 3 +-
src/include/access/rmgrlist.h | 1 -
src/include/access/xlog.h | 20 +-
src/include/access/xlog_internal.h | 6 -
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_control.h | 9 +-
src/include/catalog/pg_proc.dat | 12 -
src/include/commands/progress.h | 16 -
src/include/miscadmin.h | 10 -
src/include/postmaster/datachecksum_state.h | 28 -
src/include/postmaster/proctypelist.h | 2 -
src/include/replication/decode.h | 1 -
src/include/storage/checksum.h | 11 +-
src/include/storage/lwlocklist.h | 1 -
src/include/storage/procsignal.h | 4 -
src/include/storage/subsystemlist.h | 1 -
src/include/utils/backend_progress.h | 1 -
src/test/modules/Makefile | 1 -
src/test/modules/meson.build | 1 -
src/test/modules/test_checksums/.gitignore | 2 -
src/test/modules/test_checksums/Makefile | 36 -
src/test/modules/test_checksums/README | 30 -
src/test/modules/test_checksums/meson.build | 40 -
src/test/modules/test_checksums/t/001_basic.pl | 221 ---
src/test/modules/test_checksums/t/002_restarts.pl | 160 --
.../test_checksums/t/003_standby_restarts.pl | 288 ---
src/test/modules/test_checksums/t/004_offline.pl | 108 --
src/test/modules/test_checksums/t/005_injection.pl | 215 ---
.../modules/test_checksums/t/006_pgbench_single.pl | 285 ---
.../test_checksums/t/007_pgbench_standby.pl | 411 ----
src/test/modules/test_checksums/t/008_pitr.pl | 192 --
src/test/modules/test_checksums/t/009_fpi.pl | 64 -
.../test_checksums/t/010_backup_straddle.pl | 260 ---
.../test_checksums/t/011_standby_straddle.pl | 249 ---
.../test_checksums/t/DataChecksums/Utils.pm | 243 ---
.../modules/test_checksums/test_checksums--1.0.sql | 16 -
src/test/modules/test_checksums/test_checksums.c | 105 --
.../modules/test_checksums/test_checksums.control | 4 -
src/test/regress/expected/rules.out | 38 -
src/test/regress/expected/stats.out | 11 +-
src/tools/pgindent/typedefs.list | 6 -
82 files changed, 79 insertions(+), 6673 deletions(-)

[pg-hub-mirror[bot]]

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> via pgsql-hackers · original email

On Wed, Sep 16, 2026 at 6:06 PM Daniel Gustafsson
<dgustafsson(at)postgresql(dot)org> wrote:

Revert online data checksum transitions
It's unfortunate that this feature had to be reverted...
Anyway, thanks for all your hard work on it!!
After the revert, the buildfarm member longfin reported the test failure [1],
which seems to have been caused by the revert.
According to longfin's server log, the test triggers the following
assertion failure:
TRAP: failed Assert("(data - start) == data_size"), File:
"heaptuple.c", Line: 442, PID: 78459
...
2026-09-16 06:03:57.172 EDT [77693:4] LOG: client backend (PID

78459. was terminated by signal 6: Abort trap: 6
       2026-09-16 06:03:57.172 EDT [77693:5] DETAIL: Failed process was
       running: SELECT count(*) > 0 AS ok FROM pg_control_checkpoint();
       After the revert, pg_control_checkpoint() now returns 19 columns, but its
       pg_proc.dat entry still declares 20 output columns, including
       data_page_checksum_version. This seems to trigger the assertion failure
       in heap_form_tuple().
       We should remove data_page_checksum_version from the pg_control_checkpoint
       entry in pg_proc.dat and reduce proargmodes from 20 output columns to 19?
       Regards,
       [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=longfin&dt=2026-09-16%2010%3A01%3A48
       --
       Fujii Masao

[pg-hub-mirror[bot]]

Daniel Gustafsson <daniel(at)yesql(dot)se> via pgsql-hackers · original email

On 16 Sep 2026, at 13:01, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

On Wed, Sep 16, 2026 at 6:06 PM Daniel Gustafsson
<dgustafsson(at)postgresql(dot)org> wrote:

Revert online data checksum transitions

It's unfortunate that this feature had to be reverted...
Anyway, thanks for all your hard work on it!!

After the revert, the buildfarm member longfin reported the test failure [1],
which seems to have been caused by the revert.

According to longfin's server log, the test triggers the following
assertion failure:

TRAP: failed Assert("(data - start) == data_size"), File:
"heaptuple.c", Line: 442, PID: 78459
...
2026-09-16 06:03:57.172 EDT [77693:4] LOG: client backend (PID
78459) was terminated by signal 6: Abort trap: 6
2026-09-16 06:03:57.172 EDT [77693:5] DETAIL: Failed process was
running: SELECT count(*) > 0 AS ok FROM pg_control_checkpoint();

After the revert, pg_control_checkpoint() now returns 19 columns, but its
pg_proc.dat entry still declares 20 output columns, including
data_page_checksum_version. This seems to trigger the assertion failure
in heap_form_tuple().

We should remove data_page_checksum_version from the pg_control_checkpoint
entry in pg_proc.dat and reduce proargmodes from 20 output columns to 19?
Ugh, I thought I had tested everything but clearly missed this one. Will fix immediately when back from lunch.
./daniel

[pg-hub-mirror[bot]]

Aleksander Alekseev <aleksander(at)tigerdata(dot)com> via pgsql-hackers · original email

Hi Daniel,

[...]

We should remove data_page_checksum_version from the pg_control_checkpoint
entry in pg_proc.dat and reduce proargmodes from 20 output columns to 19?

Ugh, I thought I had tested everything but clearly missed this one. Will fix immediately when back from lunch.
I noticed that the revert was applied to REL_19_STABLE but not the
master branch. Just wanted to make sure that's the plan.
--
Best regards,
Aleksander Alekseev

[pg-hub-mirror[bot]]

Daniel Gustafsson <daniel(at)yesql(dot)se> via pgsql-hackers · original email

On 16 Sep 2026, at 14:32, Aleksander Alekseev <aleksander(at)tigerdata(dot)com> wrote:

We should remove data_page_checksum_version from the pg_control_checkpoint
entry in pg_proc.dat and reduce proargmodes from 20 output columns to 19?

Ugh, I thought I had tested everything but clearly missed this one. Will fix immediately when back from lunch.

I noticed that the revert was applied to REL_19_STABLE but not the
master branch. Just wanted to make sure that's the plan.
That's indeed the plan. The remaining issue that a user can, under the right
set of circumstances, get a false positive page verification in an orphaned
file failure during base backup. The warning is harmless for data integrity,
but is non-trivial for the user to resolve since we don't provide any tools for
dealing with orphaned files. Fixing this in master will reduce churn.
--
Daniel Gustafsson

[pg-hub-mirror[bot]]

Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> via pgsql-hackers · original email

Daniel Gustafsson <dgustafsson(at)postgresql(dot)org> wrote:

Ugh, I thought I had tested everything but clearly missed this one. Will fix immediately when back from lunch.
While you are at it: the revert removes RM_XLOG2_ID and its records but
leaves XLOG_PAGE_MAGIC at 0xD121 on REL_19_STABLE. 0xD121 was set on
2026-07-15 by b01c31e, after the feature had added XLOG2 on
2026-04-03, and it is what 19beta3 shipped with. So WAL written by
19beta3 and WAL written by 19.0 now carry the same magic but are not
the same format.
To check, I enabled checksums online on 19beta3 and read the WAL with
the pg_waldump of REL_19_STABLE at d8408e8:
19beta3's pg_waldump -r XLOG2:
rmgr: XLOG2 ... lsn: 0/0157F5A8 ... desc: CHECKSUMS inprogress-on
rmgr: XLOG2 ... lsn: 0/02DC9AD8 ... desc: CHECKSUMS on
REL_19_STABLE's pg_waldump, same segments:
pg_waldump: error: could not find a valid record after 0/0157F5A8:
invalid resource manager ID 22 at 0/0157F5A8
REL_19_STABLE's pg_waldump with XLOG_PAGE_MAGIC bumped:
pg_waldump: error: could not find a valid record after 0/0157F5A8:
invalid magic number D121 in WAL segment 000000010000000000000001,
LSN 0/01000000, offset 0
The server itself is not at risk, since PG_CONTROL_VERSION was bumped
and 19.0 refuses a beta3 data directory; it is only tools reading
archived beta3 WAL that cannot tell the formats apart. But the revert
of 0d3dba3 on this branch (01a80f0) bumped the magic for the
same reason, so it looks like this one should too. Note that master is
already at 0xD122, for a different format.
The script is attached, in case it is useful.
Regards,
Manu

[pg-hub-mirror[bot]]

Daniel Gustafsson <daniel(at)yesql(dot)se> via pgsql-hackers · original email

On 16 Sep 2026, at 14:58, Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> wrote:
The server itself is not at risk, since PG_CONTROL_VERSION was bumped
and 19.0 refuses a beta3 data directory; it is only tools reading
archived beta3 WAL that cannot tell the formats apart. But the revert
of 0d3dba3 on this branch (01a80f0) bumped the magic for the
same reason, so it looks like this one should too.
Thanks for the report, will include when fixing.
--
Daniel Gustafsson

[pg-hub-mirror[bot]]

Heikki Linnakangas <hlinnaka(at)iki(dot)fi> via pgsql-hackers · original email

On 16/09/2026 16:04, Daniel Gustafsson wrote:

On 16 Sep 2026, at 14:58, Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> wrote:

The server itself is not at risk, since PG_CONTROL_VERSION was bumped
and 19.0 refuses a beta3 data directory; it is only tools reading
archived beta3 WAL that cannot tell the formats apart. But the revert
of 0d3dba3 on this branch (01a80f0) bumped the magic for the
same reason, so it looks like this one should too.

Thanks for the report, will include when fixing.
Also, the ControlFile->data_checksum_version_init is now never set.

• Heikki

[pg-hub-mirror[bot]]

Daniel Gustafsson <daniel(at)yesql(dot)se> via pgsql-hackers · original email

On 16 Sep 2026, at 16:08, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

On 16/09/2026 16:04, Daniel Gustafsson wrote:

On 16 Sep 2026, at 14:58, Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> wrote:
The server itself is not at risk, since PG_CONTROL_VERSION was bumped
and 19.0 refuses a beta3 data directory; it is only tools reading
archived beta3 WAL that cannot tell the formats apart. But the revert
of 0d3dba3 on this branch (01a80f0) bumped the magic for the
same reason, so it looks like this one should too.
Thanks for the report, will include when fixing.

Also, the ControlFile->data_checksum_version_init is now never set.
This patch is as difficult to back out as it was to get in. I've been trying
to save a few small generic checksum bugfixes from the revert but that also
made it a lot more complicated. Will fix this one as well in the follow-up
fixup I have staged for later today.
--
Daniel Gustafsson

[pg-hub-mirror[bot]]

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> via pgsql-hackers · original email

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:

On 16 Sep 2026, at 17:49, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
I can poke into it on longfin's host if that would help.
That would be great, thanks.
What seems to be happening is
(1) pg_control_checkpoint()'s local values[] and nulls[] arrays
are now of length 19.
(2) But the tupdesc that get_call_result_type() builds from the
pg_proc entry still has 20 columns.
(3) Therefore, heap_form_tuple sees a wrong value for the 19th column
(which does no great damage, since it's still a pass-by-value type)
and then fetches complete garbage from values[19] and nulls[19].
Depending on the stack layout and the phase of the moon, nulls[19]
might read as zero or it might not. Either case is fine so long as
heap_compute_data_size and heap_fill_tuple/fill_val interpret it the
same way. However, what I'm seeing as I trace through the crash on
longfin is that nulls[19] has a value that's not either 0 or 1 (not
too surprising), and somehow heap_compute_data_size is interpreting
that as "true" while fill_val is reading it as "false". According
to my understanding of the C standard, fill_val is in the wrong here.
I looked at the generated assembly code and could not spot exactly
what the problem is; it seems to be testing the byte value with
.loc 0 301 7 is_stmt 1 ## heaptuple.c:301:7
testb %al, %al
je LBB3_6
which certainly looks like it ought to treat all nonzero values alike.
Anyway this seems to be a code generation quirk of certain versions of
clang. It's probably not worth chasing further.
regards, tom lane

[pg-hub-mirror[bot]]

Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> via pgsql-hackers · original email

On Wed, Sep 16, 2026 at 10:14 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

However, what I'm seeing as I trace through the crash on
longfin is that nulls[19] has a value that's not either 0 or 1 (not
too surprising), and somehow heap_compute_data_size is interpreting
that as "true" while fill_val is reading it as "false". According
to my understanding of the C standard, fill_val is in the wrong here.
Is the _Bool representation allowed to contain anything but zero/false
or one/true? I thought that was undefined behavior. (IIRC, mixing C++
bool with our C89-era bool definition suffered from similar UB
excitement.)
--Jacob

[pg-hub-mirror[bot]]

Bryan Green <dbryan(dot)green(at)gmail(dot)com> via pgsql-hackers · original email

If we are pulling in stdbool.h and this is C11...that is undefined behavior
and because it wasn't validly initialized it is an indeterminate-value
read...which means for optimization that the compiler is within its right
to assume it is a 0 or 1 and could test it in different ways depending on
the surrounding code (nonzero test, compare to 1, etc).
On Wed, Sep 16, 2026 at 12:34 PM Jacob Champion <
jacob(dot)champion(at)enterprisedb(dot)com> wrote:

On Wed, Sep 16, 2026 at 10:14 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

However, what I'm seeing as I trace through the crash on
longfin is that nulls[19] has a value that's not either 0 or 1 (not
too surprising), and somehow heap_compute_data_size is interpreting
that as "true" while fill_val is reading it as "false". According
to my understanding of the C standard, fill_val is in the wrong here.

Is the _Bool representation allowed to contain anything but zero/false
or one/true? I thought that was undefined behavior. (IIRC, mixing C++
bool with our C89-era bool definition suffered from similar UB
excitement.)

--Jacob

[pg-hub-mirror[bot]]

Bryan Green <dbryan(dot)green(at)gmail(dot)com> via pgsql-hackers · original email

This is the goto example:
https://markshroyer.com/2012/06/c-both-true-and-false/
On Wed, Sep 16, 2026 at 12:39 PM Bryan Green <dbryan(dot)green(at)gmail(dot)com> wrote:

If we are pulling in stdbool.h and this is C11...that is undefined
behavior and because it wasn't validly initialized it is an
indeterminate-value read...which means for optimization that the compiler
is within its right to assume it is a 0 or 1 and could test it in different
ways depending on the surrounding code (nonzero test, compare to 1, etc).

On Wed, Sep 16, 2026 at 12:34 PM Jacob Champion <
jacob(dot)champion(at)enterprisedb(dot)com> wrote:

On Wed, Sep 16, 2026 at 10:14 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

However, what I'm seeing as I trace through the crash on
longfin is that nulls[19] has a value that's not either 0 or 1 (not
too surprising), and somehow heap_compute_data_size is interpreting
that as "true" while fill_val is reading it as "false". According
to my understanding of the C standard, fill_val is in the wrong here.

Is the _Bool representation allowed to contain anything but zero/false
or one/true? I thought that was undefined behavior. (IIRC, mixing C++
bool with our C89-era bool definition suffered from similar UB
excitement.)

--Jacob

[pg-hub-mirror[bot]]

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> via pgsql-hackers · original email

On Wed, Sep 16, 2026 at 8:08 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:

Ugh, I thought I had tested everything but clearly missed this one. Will fix immediately when back from lunch.
Here are another comments:
data_checksum_version field in xl_checkpoint_redo struct should be removed?
Otherwise, it seems to be included in WAL records unnecessarily even though
it is no longer used. No?

•   database (or on a shared object), or NULL if data checksums are
  

•   disabled.
  

•  </para></entry>
  

•   database (or on a shared object).  Detected failures are not reset if
  

•   the <xref linkend="guc-data-checksums"/> setting changes.  Clusters
  

•   which are initialized without data checksums will show this as
  

•   <literal>0</literal>. In <productname>PostgreSQL</productname> version
  

•   18 and earlier, this was set to <literal>NULL</literal> for clusters
  

•   with data checksums disabled.
  

•  </para>
  

• </entry>
  

...

•   this database (or on a shared object), or NULL if data checksums are
  

•   disabled.
  

•   this database (or on a shared object). Last failure is reported
  

•   regardless of the <xref linkend="guc-data-checksums"/> setting.
  

These updates to monitoring.sgml from commit f19c0ec also should be
reverted? checksum_failures and checksum_last_failure seem to return
NULL again when data checksums are disabled, so their descriptions would
otherwise be incorrect.
checksum_enable_offline() and checksum_disable_offline() in
Cluster.pm also seem to be unused now. Should they be removed as well?
Regards,

Fujii Masao

[pg-hub-mirror[bot]]

A patch is now available as PR #405.