Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted.

On Fri, Dec 8, 2017 at 4:13 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Dec 8, 2017 at 5:38 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> After off-discussion with Fujii-san, I've updated the comment of why
>> we should disallow interrupts before setting/cleanup the session-level
>> lock. Please review it.
>
> + /*
> + * Set session-level lock. If we allow interrupts before setting
> + * session-level lock, we could call callbacks with an inconsistent
> + * state. To avoid calling CHECK_FOR_INTERRUPTS by LWLockReleaseClearVar
> + * which is called by WALInsertLockRelease before changing the backup
> + * state we change it while holding the WAL insert lock.
> + */
> So you are just adding the reference to WALInsertLockRelease.. Instead
> of writing the function names for LWLocks, I would just write "To
> avoid calling CHECK_FOR_INTERRUPTS which can happen when releasing a
> LWLock" and be done with it. There is no point to list a full function
> dependency list, which could change in the future with static routines
> of lwlock.c.

I think it's actually good to be explicit here. I looked at this
patch a bit last week and had great difficulty understanding how the
CHECK_FOR_INTERRUPTS() could happen.

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