Horton

Re: Race conditions in 019_replslot_limit.pl

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, hlinnaka@iki.fi, pgsql-hackers@postgresql.org
Date: 2022-02-17T04:32:52Z
Lists: pgsql-hackers 
Hi,

On 2022-02-16 22:58:19 -0500, Tom Lane wrote:
> I wonder though if an exiting walsender would always take that path.

You're right - the CFI() in PostgresMain(), several replication commands, are
not at all guaranteed to be in a transaction when throwing a FATAL.

I don't quite see where we could be throwing an error with a relevant lwlock
held yet though.


> Maybe it'd be good if we released all LWLocks even if we don't think we're
> in a transaction?

Yea, probably a good idea. I think there's a few hooks could that could
conceivably be in trouble otherwise.


> More generally, it wasn't obvious to me why you thought that
> BaseInit was a good spot to place the on_shmem_exit call.

> It obviously can't be *before* that because of the pgstats
> dependency, but maybe it should be later?

Yea, I agree, it probably could be done later. It's not super obvious when
though. If e.g. somebody wrote a non-database connected bgworker to
synchronize slot state with another node (for failover, there's even patches
for it), it'd be just after BaseInit(). Additionally, it was previously in
ProcKill(), so moving it something registered early-ish seemed sensible.

Greetings,

Andres Freund

Commits

  1. Revert 019_replslot_limit.pl related debugging aids.

  2. Don't fail for > 1 walsenders in 019_replslot_limit, add debug messages.

  3. Add retries for further investigation of 019_replslot_limit.pl failures.

  4. Add further debug info to help debug 019_replslot_limit.pl failures.

  5. Add temporary debug info to help debug 019_replslot_limit.pl failures.

  6. Move replication slot release to before_shmem_exit().

  7. Server-side fix for delayed NOTIFY and SIGTERM processing.