Re: Online enable/disable data checksums functions return success even when the launcher fails to start

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Online enable/disable data checksums functions return success even when the launcher fails to start
Date: 2026-08-29 05:09:00
Message-ID: CALj2ACXNOXtS8QOtZ9y+_QEi5Dg4H8y8tyk_ngeabZq7H_ZEaw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Aug 27, 2026 at 10:15 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
> > On 27 Aug 2026, at 19:04, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> > pg_enable_data_checksums() and pg_disable_data_checksums() start a
> > launcher background worker via StartDataChecksumsWorkerLauncher() and
> > then return immediately. The function errors out if registration
> > fails, but it does not check whether the launcher actually started
> > after that. If the postmaster registers the launcher but then fails to
> > fork it (e.g., fork failure under memory pressure), the SQL function
> > still returns success, the launcher never runs.
>
> The functions return void and were designed to initiate processing but not
> track any level of progress, since processing can take a long time.

It silently ignores the fork failure. One can still look at the
progress report or server logs to find that, but I think having this
fixed in a simple way is better.

> > The caller gets no indication that the requested operation did not happen. I reproduced
> > this with an induced fork failure, so I think we need to tighten this
> > for both PG19 and HEAD branches.
>
> There is also no indication of the operation succeeding from the functions,
> pg_stat_activity has the details for this. It's too late to change the
> function signature for PG19.
>
> > Fix would be to check
> > GetBackgroundWorkerPid()/WaitForBackgroundWorkerStartup() and error
> > out when the worker has not started. If okay, I can send a patch.
>
> I'm not convinced there is much value in adding such complexity as it would
> have to handle more cases than that to be useful. There is
> pg_stat_progress_data_checksums which can be queried for details on the
> processing.
>
> It's too late for v19 (in more ways than one perhaps), but feel free to post a
> suggestion for HEAD and we can evaluate it from there.

Here's my first attempt at fixing this without changing the function
return types. In the best case, waiting for the background worker to
start is very short, and if any failure is detected in forking the
worker, these functions error out (similar to
apw_start_leader_worker()). I still think this needs to be tightened
in PG19 as well, but I'm fine with HEAD only. Please have a look at
the attached patch.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v1-0001-Report-an-error-when-the-data-checksums-worker-fa.patch application/octet-stream 2.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Alexander Lakhin 2026-08-29 05:00:00 Re: Changing the state of data checksums in a running cluster