Thread: pgsql: Fix minor problems with non-exclusive backup cleanup.
Fix minor problems with non-exclusive backup cleanup. The previous coding imagined that it could call before_shmem_exit() when a non-exclusive backup began and then remove the previously-added handler by calling cancel_before_shmem_exit() when that backup ended. However, this only works provided that nothing else in the system has registered a before_shmem_exit() hook in the interim, because cancel_before_shmem_exit() is documented to remove a callback only if it is the latest callback registered. It also only works if nothing can ERROR out between the time that sessionBackupState is reset and the time that cancel_before_shmem_exit(), which doesn't seem to be strictly true. To fix, leave the handler installed for the lifetime of the session, arrange to install it just once, and teach it to quietly do nothing if there isn't a non-exclusive backup in process. This was originally committed to master as 303640199d0436c5e7acdf50b837a027b5726594, but I did not back-patch at the time because the consequences were minor. However, now there's been a second report of this causing trouble with a slightly different test case than the one I reported originally, so now I'm back-patching as far as v11 where JIT was introduced. Patch by me, reviewed by Kyotaro Horiguchi, Michael Paquier (who preferred a different approach, but got outvoted), Fujii Masao, and Tom Lane, and with comments by various others. New problem report from Bharath Rupireddy. Discussion: http://postgr.es/m/CA+TgmobMjnyBfNhGTKQEDbqXYE3_rXWpc4CM63fhyerNCes3mA@mail.gmail.com Discussion: http://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com Branch ------ REL_12_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/bcbc27251d35336a6442761f59638138a772b839 Modified Files -------------- src/backend/access/transam/xlog.c | 32 +++++++++++++++++++++++++++++--- src/backend/access/transam/xlogfuncs.c | 17 ++--------------- src/backend/replication/basebackup.c | 16 ++-------------- src/include/access/xlog.h | 3 ++- 4 files changed, 35 insertions(+), 33 deletions(-)
Robert Haas <rhaas@postgresql.org> writes:
> Fix minor problems with non-exclusive backup cleanup.
Seems like there's an extra space in this error message in
do_pg_abort_backup:
+ if (emit_warning)
+ ereport(WARNING,
+ (errmsg("aborting backup due to backend exiting before pg_stop_back up was called")));
Also, while I'm bugging you: it wasn't quite clear from the thread
whether this is alone sufficient to fix the user-visible issue
reported by Bharath Rupireddy, or whether we still need to change
the JIT logic as well to do that.
regards, tom lane
On Thu, Aug 6, 2020 at 2:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <rhaas@postgresql.org> writes:
> > Fix minor problems with non-exclusive backup cleanup.
>
> Seems like there's an extra space in this error message in
> do_pg_abort_backup:
>
> + if (emit_warning)
> + ereport(WARNING,
> + (errmsg("aborting backup due to backend exiting before pg_stop_back up was called")));
Ah, crap. Peter fixed that before, and I forgot to include it when
back-patching.
> Also, while I'm bugging you: it wasn't quite clear from the thread
> whether this is alone sufficient to fix the user-visible issue
> reported by Bharath Rupireddy, or whether we still need to change
> the JIT logic as well to do that.
I believe it suffices, but see also the commit I pushed to master.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Aug 6, 2020 at 2:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Also, while I'm bugging you: it wasn't quite clear from the thread
>> whether this is alone sufficient to fix the user-visible issue
>> reported by Bharath Rupireddy, or whether we still need to change
>> the JIT logic as well to do that.
> I believe it suffices, but see also the commit I pushed to master.
Yeah, I saw that later. But thanks for confirming.
regards, tom lane