Horton

Re: Race conditions in 019_replslot_limit.pl

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, hlinnaka@iki.fi, pgsql-hackers@postgresql.org
Date: 2022-02-18T23:49:14Z
Lists: pgsql-hackers 
Andres Freund <andres@anarazel.de> writes:
> On 2022-02-18 18:15:21 -0500, Tom Lane wrote:
>> Perhaps it'd be sensible to do this only in debugging (ie Assert)
>> builds?

> That seems not great, because it pretty clearly can lead to hangs, which is
> problematic in tests too. What about using pq_flush_if_writable()? In nearly
> all situations that'd still push the failure to the client.

That'd be okay by me.

> We'd also need to add pq_endmessage_noblock(), because the pq_endmessage()
> obviously tries to send (as in the backtrace upthread) if the output buffer is
> large enough, which it often will be in walsender.

I don't see that as "obvious".  If we're there, we do not have an
error situation.

> I guess we could try to flush in a blocking manner sometime later in the
> shutdown sequence, after we've released resources? But I'm doubtful it's a
> good idea, we don't really want to block waiting to exit when e.g. the network
> connection is dead without the TCP stack knowing.

I think you are trying to move in the direction of possibly exiting
without ever sending at all, which does NOT seem like an improvement.

			regards, tom lane

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.