Fujii Masao <[email protected]> wrote: > 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?
Agreed. It is also never assigned now: c05d5ce1236 removed the line in CreateCheckPoint() that set it, but the whole struct is still registered, so the main data of each XLOG_CHECKPOINT_REDO record is 8 bytes, 4 of them unset. Before f19c0eccae9 it was 4 bytes. In my runs on REL_19_STABLE at 4a9a6c5a69c the unset bytes happened to be zero. > These updates to monitoring.sgml from commit f19c0eccae9 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. Yes, both pg_stat_get_db_checksum_failures() and pg_stat_get_db_checksum_last_failure() return NULL when DataChecksumsEnabled() is false, as they did before f19c0eccae9. > checksum_enable_offline() and checksum_disable_offline() in > Cluster.pm also seem to be unused now. Should they be removed as well? They have no callers left in the tree. To check whether anything else was left behind, I went through the 30 commits listed in c05d5ce1236 mechanically: the identifiers and the non-trivial lines they added that are still in REL_19_STABLE and were not there before f19c0eccae9. Apart from what the revert keeps on purpose (the checksum state enum, data_checksum_version_init, the zeroing note in page verification and the rewritten docs section), these three are the only ones left. In case it saves Daniel some time, the attached patch does the three, against REL_19_STABLE. The changed parts match the text before f19c0eccae9. It builds without warnings, make check passes, the redo records are back to 4 bytes of data, and the two readers of that data, pg_waldump and the WAL summarizer (with summarize_wal on), work as before. I have not touched XLOG_PAGE_MAGIC. Nothing has been released since 4a9a6c5a69c bumped it, but pg_waldump or a summarizer built before this patch would copy 8 bytes out of a 4-byte record, so that is your call. Regards, Manu
From d3d11a61172d6689ffaab7f2c7ed30b7cb66d237 Mon Sep 17 00:00:00 2001 From: Manu <[email protected]> Date: Thu, 17 Sep 2026 10:24:01 -0300 Subject: [PATCH] Remove remaining leftovers of the online checksums revert Commit c05d5ce1236 removed the code that set and read the data_checksum_version member of xl_checkpoint_redo, but not the member itself, so every XLOG_CHECKPOINT_REDO record still carried four bytes that are never assigned. Remove it; the record data is back to the four bytes it had before f19c0eccae9. Also restore the descriptions of checksum_failures and checksum_last_failure, which are NULL again when data checksums are disabled, and remove the checksum_enable_offline() and checksum_disable_offline() test helpers, which have no callers left. Reported-by: Fujii Masao <[email protected]> --- doc/src/sgml/monitoring.sgml | 15 ++++------ src/include/access/xlog_internal.h | 1 - src/test/perl/PostgreSQL/Test/Cluster.pm | 36 ------------------------ 3 files changed, 5 insertions(+), 47 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 86982079362..fe31b7b62ec 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3902,14 +3902,9 @@ description | Waiting for a newly initialized WAL file to reach durable storage </para> <para> Number of data page checksum failures detected in this - 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> + database (or on a shared object), or NULL if data checksums are + disabled. + </para></entry> </row> <row> @@ -3918,8 +3913,8 @@ description | Waiting for a newly initialized WAL file to reach durable storage </para> <para> Time at which the last data page checksum failure was detected in - this database (or on a shared object). Last failure is reported - regardless of the <xref linkend="guc-data-checksums"/> setting. + this database (or on a shared object), or NULL if data checksums are + disabled. </para></entry> </row> diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index 6639dc19e32..7503c49f203 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -308,7 +308,6 @@ typedef struct xl_end_of_recovery typedef struct xl_checkpoint_redo { int wal_level; - uint32 data_checksum_version; } xl_checkpoint_redo; /* diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 366519e22b5..da8d5516b53 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -3992,42 +3992,6 @@ sub advance_wal } } -=item $node->checksum_enable_offline() - -Enable data page checksums in an offline cluster with B<pg_checksums>. The -caller is responsible for ensuring that the cluster is in the right state for -this operation. - -=cut - -sub checksum_enable_offline -{ - my ($self) = @_; - - print "# Enabling checksums in \"$self->data_dir\"\n"; - PostgreSQL::Test::Utils::system_or_bail('pg_checksums', '-D', - $self->data_dir, '-e'); - return; -} - -=item $node->checksum_disable_offline() - -Disable data page checksums in an offline cluster with B<pg_checksums>. The -caller is responsible for ensuring that the cluster is in the right state for -this operation. - -=cut - -sub checksum_disable_offline -{ - my ($self) = @_; - - print "# Disabling checksums in \"$self->data_dir\"\n"; - PostgreSQL::Test::Utils::system_or_bail('pg_checksums', '-D', - $self->data_dir, '-d'); - return; -} - =pod =back -- 2.55.0
