Re: [HACKERS] Backend crash on non-exclusive backup cancel - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [HACKERS] Backend crash on non-exclusive backup cancel
Date
Msg-id CAB7nPqQD9X0Bbzqy4YyUT_xrxEseaHBJ6mghiA4VvJFVJCZ-5Q@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Backend crash on non-exclusive backup cancel  (Anastasia Lubennikova <lubennikovaav@gmail.com>)
Responses Re: [HACKERS] Backend crash on non-exclusive backup cancel  (Anastasia Lubennikova <lubennikovaav@gmail.com>)
Re: [HACKERS] Backend crash on non-exclusive backup cancel  (David Steele <david@pgmasters.net>)
Re: [HACKERS] Backend crash on non-exclusive backup cancel  (Andres Freund <andres@anarazel.de>)
Re: [HACKERS] Backend crash on non-exclusive backup cancel  (Teodor Sigaev <teodor@sigaev.ru>)
List pgsql-hackers
On Wed, Mar 15, 2017 at 12:27 AM, Anastasia Lubennikova
<lubennikovaav@gmail.com> wrote:
> As far as I understand, in this thread were discussed two bugs of pg_stop_backup().
> Thanks to the clear descriptions above, I easily reproduced both of them.
>
> BUG#1:
> Server crashes on assertion on call of pg_stop_backup(false) after interrupted call of pg_stop_backup(false).
> TRAP: FailedAssertion("!(XLogCtl->Insert.nonExclusiveBackups > 0)", File: "xlog.c", Line: 10747)
>
> BUG#2:
> Failure to start an exclusive backup with the same name, if previous exclusive backup was stopped in another
session.
>
> Both problems seem to be fixed with patch "backup-session-locks-fixes.patch".
> Speaking of the patch itself, I have a question: shouldn't we also update sessionBackupState
> in pg_stop_backup_callback() along with XLogCtl->Insert.exclusiveBackupState?

No, that's not necessary. sessionBackupState is not changed until the
code path where pg_stop_backup_callback() is active, and remains
unchanged until it gets deactivated.

> And couple of minor notes:
> 1) + * Routines to starting stop, and get status of a base backup
> Probably should be: + * Routines to start, stop and get status of a base backup
> And also this comment should be moved below the enum.
>
> 2) This is used in parallel of the shared memory status
> s/ in parallel of/ in parallel with

Agreed on both points.
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Kuntal Ghosh
Date:
Subject: Re: [HACKERS] WAL Consistency checking for hash indexes
Next
From: Peter van Hardenberg
Date:
Subject: [HACKERS] Defaulting psql to ON_ERROR_ROLLBACK=interactive