| From: | Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, Daniel Gustafsson <dgustafsson(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgsql: Revert online data checksum transitions |
| Date: | 2026-09-17 13:27:58 |
| Message-ID: | CA+bCEdA4QxMc2KYqfuEZofPBYjx_wHLthfk4iDFVRCn+hoF1mg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers pgsql-hackers |
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: 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
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Remove-remaining-leftovers-of-the-online-checksums-r.patch | text/x-patch | 4.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-09-17 13:47:33 | pgsql: Change GEQO fitness comparisons to consider disabled_nodes. |
| Previous Message | Daniel Gustafsson | 2026-09-17 12:58:24 | pgsql: Test output fixes for oauth validator test suite |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-17 13:31:44 | Re: ON EMPTY clause for aggregate and window functions |
| Previous Message | Robert Haas | 2026-09-17 13:16:11 | Re: ON EMPTY clause for aggregate and window functions |