non-exclusive backup cleanup is mildly broken - Mailing list pgsql-hackers

From Robert Haas
Subject non-exclusive backup cleanup is mildly broken
Date
Msg-id CA+TgmobMjnyBfNhGTKQEDbqXYE3_rXWpc4CM63fhyerNCes3mA@mail.gmail.com
Whole thread Raw
Responses Re: non-exclusive backup cleanup is mildly broken  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List 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

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: [Proposal] Level4 Warnings show many shadow vars
Next
From: Alex Adriaanse
Date:
Subject: Re: Corruption with duplicate primary key