Skip to content

pgsql: Revert online data checksum transitions - #405

Open
pg-hub-mirror[bot] wants to merge 1 commit into
masterfrom
pg-hub/mirror-patch-481128c0a1e64e60
Open

pg-hub-mirror[bot] wants to merge 1 commit into
masterfrom
pg-hub/mirror-patch-481128c0a1e64e60

Conversation

@pg-hub-mirror

@pg-hub-mirror pg-hub-mirror Bot commented Sep 17, 2026

Copy link
Copy Markdown

Read-only mirror. Reply and review on pgsql-hackers; activity here is not sent upstream.

  • Original author: Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com>
  • Mailing list: pgsql-hackers
  • Message-ID: CA+bCEdA4QxMc2KYqfuEZofPBYjx_wHLthfk4iDFVRCn+hoF1mg@mail.gmail.com
  • Original email

Patch files:


Fujii Masao <masao(dot)fujii(at)gmail(dot)com> 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: c05d5ce 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 f19c0ec it was 4 bytes. In my runs
on REL_19_STABLE at 4a9a6c5 the unset bytes happened to be zero.
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.
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 f19c0ec.
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 c05d5ce mechanically: the identifiers and the
non-trivial lines they added that are still in REL_19_STABLE and were
not there before f19c0ec. 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
f19c0ec. 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
4a9a6c5 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

Commit c05d5ce 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 f19c0ec.

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 <masao.fujii@gmail.com>
@pg-hub-mirror

pg-hub-mirror Bot commented Sep 17, 2026

Copy link
Copy Markdown
Author

Earlier design discussion: Discussion #366

@pg-hub-mirror pg-hub-mirror Bot added source:pgsql-hackers Mirrored from pgsql-hackers type:patch Mail thread contains a PostgreSQL patch area:wal Write-ahead logging and recovery area:sql SQL language or commands area:docs Documentation labels Sep 17, 2026
@pg-hub-mirror pg-hub-mirror Bot locked and limited conversation to collaborators Sep 17, 2026
@pg-hub-mirror pg-hub-mirror Bot unlocked this conversation Sep 17, 2026
@pg-hub-mirror

pg-hub-mirror Bot commented Sep 17, 2026

Copy link
Copy Markdown
Author

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

On 17 Sep 2026, at 15:27, Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> wrote:
In case it saves Daniel some time, the attached patch does the three,
against REL_19_STABLE.
Thanks, I also went over the commits today so I'll compare notes before
applying to v19 later today.
--
Daniel Gustafsson

@pg-hub-mirror pg-hub-mirror Bot locked and limited conversation to collaborators Sep 17, 2026
@pg-hub-mirror pg-hub-mirror Bot unlocked this conversation Sep 17, 2026
@pg-hub-mirror

pg-hub-mirror Bot commented Sep 17, 2026

Copy link
Copy Markdown
Author

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

On 17 Sep 2026, at 18:54, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:



On 17 Sep 2026, at 15:27, Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> wrote:

In case it saves Daniel some time, the attached patch does the three,
against REL_19_STABLE.

Thanks, I also went over the commits today so I'll compare notes before
applying to v19 later today.
I’ve applied this now. I originally kept the Test code in place ro minimize backpatching conflict risks there but after thinking further I ended up removing it as well.
./daniel

@pg-hub-mirror pg-hub-mirror Bot locked and limited conversation to collaborators Sep 17, 2026
@pg-hub-mirror pg-hub-mirror Bot added the area:testing Tests and buildfarm label Sep 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area:docs Documentation area:sql SQL language or commands area:testing Tests and buildfarm area:wal Write-ahead logging and recovery source:pgsql-hackers Mirrored from pgsql-hackers type:patch Mail thread contains a PostgreSQL patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant