non-exclusive backup cleanup is mildly broken

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: non-exclusive backup cleanup is mildly broken
Date: 2019-12-11 22:32:05
Message-ID: CA+TgmobMjnyBfNhGTKQEDbqXYE3_rXWpc4CM63fhyerNCes3mA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While reviewing the first patch in Asif Rehman's series of patches for
parallel backup over at
http://postgr.es/m/CADM=Jeg3ZN+kPQpiSfeWCXr=xgpLrq4cBQE5ZviUxygKq3VqiA@mail.gmail.com
I discovered that commit 7117685461af50f50c03f43e6a622284c8d54694
introduced a use of cancel_before_shmem_exit which falsifies the
comment for that function. So you can cause a spurious WARNING in the
logs by doing something like this, with max_prepared_transactions set
to a non-zero value:

select pg_start_backup('one', false, false);
begin;
prepare transaction 'nothing';
select pg_stop_backup(false);
\q

in the server log:
WARNING: aborting backup due to backend exiting before pg_stop_backup
was called

And you can cause an assertion failure like this:

select pg_start_backup('one', false, false);
begin;
prepare transaction 'nothing';
select pg_stop_backup(false);
select pg_start_backup('two');
\q

We've discussed before the idea that it might be good to remove the
limitation that before_shmem_exit() can only remove the
most-recently-added callback, which would be one way to fix this
problem, but I'd like to propose an alternative solution which I think
will work out more nicely for the patch mentioned above: don't use
cancel_before_shmem_exit, and just leave the callback registered for
the lifetime of the backend. That requires some adjustment of the
callback, since it needs to tolerate exclusive backup mode being in
effect.

The attached patch takes that approach. Thoughts welcome on (1) the
approach and (2) whether to back-patch. I think there's little doubt
that this is formally a bug, but it's a pretty minor one.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
0001-Remove-misuse-of-cancel_before_shmem_exit.patch application/octet-stream 5.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alex Adriaanse 2019-12-11 23:42:45 Re: Corruption with duplicate primary key
Previous Message Stephen Frost 2019-12-11 21:44:55 Re: [Proposal] Level4 Warnings show many shadow vars