| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | r314tive <tanswis42(at)gmail(dot)com> |
| Cc: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: datachecksums: handle invalid and dropped databases during enable |
| Date: | 2026-07-29 12:24:25 |
| Message-ID: | 6C983F54-54EE-4F0E-BBF3-6D5F6FAED0C9@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 29 Jul 2026, at 14:00, r314tive <tanswis42(at)gmail(dot)com> wrote:
> While checking commit 51f55b13a4d, I noticed a regression in repeated
> calls to pg_enable_data_checksums().
>
> Before that commit, calling pg_enable_data_checksums() after checksums had
> reached the "on" state succeeded as a no-op. The existing basic TAP test
> also documents that behavior.
>
> 51f55b13a4d added ErrorOnInvalidDatabases() before
> StartDataChecksumsWorkerLauncher(). If an invalid database exists, the
> same repeated call now fails even though no database processing is needed:
>
> ERROR: cannot enable data checksums in a cluster with invalid database
> "baddb"
I'm not sure that's inherently wrong though, if the precondition for an
operation isn't met then the operation cannot succeed and erroring out is the
right option. If anything, wouldn't it be better to expand the errhint to
mention that any currently running checksum enabling will fail unless the
database is dropped?
> The attached v1 patch restores that no-op, but does not use a bare
> DataChecksumsOn() early return. Such a return could lose a pending disable
> request and violate the existing last-request-wins behavior.
>
> Instead, the patch makes the no-op decision under
> DataChecksumsWorkerLock and records the requested enable operation and cost
> settings atomically. If a disable operation is already running, it keeps
> the invalid-database preflight because the subsequent enable may need to
> process every database.
I don't think duplicating this logic from StartDataChecksumsWorkerLauncher is
the right move, enable_data_checksums should IMO only validate parameters and
preconditions and leave the decisionmaking to StartDataChecksumsWorkerLauncher.
--
Daniel Gustafsson
| From | Date | Subject | |
|---|---|---|---|
| Next Message | wenhui qiu | 2026-07-29 12:47:49 | Re: Prove a NOT IN's left-hand expressions non-nullable from quals |
| Previous Message | Amit Kapila | 2026-07-29 12:18:46 | Re: Race between pg_dump and ALTER SEQUENCE can cause read failure |