Re: walsender performance regression due to logical decoding on standby changes

Hi,

On 2023-05-24 05:53:51 +0000, Zhijie Hou (Fujitsu) wrote:
> On Tuesday, May 23, 2023 1:53 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2023-05-22 12:15:07 +0000, Zhijie Hou (Fujitsu) wrote:
> > > About "a backend doing logical decoding", do you mean the case when a
> > user
> > > start a backend and invoke pg_logical_slot_get_changes() to do the logical
> > > decoding ? If so, it seems the logical decoding in a backend won't be waked
> > up
> > > by startup process because the backend won't be registered as a walsender
> > so
> > > the backend won't be found in WalSndWakeup().
> >
> > I meant logical decoding happening inside a walsender instance.
> >
> >
> > > Or do you mean the deadlock between the real logical walsender and startup
> > > process ? (I might miss something) I think the logical decoding doesn't lock
> > > the target user relation when decoding because it normally can get the
> > needed
> > > information from WAL.
> >
> > It does lock catalog tables briefly. There's no guarantee that such locks are
> > released immediately. I forgot the details, but IIRC there's some outfuncs
> > (enum?) that intentionally delay releasing locks till transaction commit.
>
> Thanks for the explanation !
>
> I understand that the startup process can take lock on the catalog(when
> replaying record) which may conflict with the lock in walsender.
>
> But in walsender, I think we only start transaction after entering
> ReorderBufferProcessTXN(), and the transaction started here will be released
> soon after processing and outputting the decoded transaction's data(as the
> comment in ReorderBufferProcessTXN() says:" all locks acquired in here to be
> released, not reassigned to the parent and we do not want any database access
> have persistent effects.").

It's possible that there's no immediate danger - but I wouldn't want to bet on
it staying that way. Think about things like streaming out large transactions
etc. There also are potential dangers outside of plain things like locks -
there could be a recovery conflicts on the snapshot or such (although that
normally should be prevented via hot_standby_feedback or such).

Even if there's no hard deadlock issue - not waking up a
walsender-in-logical-decoding that's currently waiting because (replay_count %
N) != 0 means that the walsender might be delayed for quite a while - there
might not be any further records from the primary in the near future.

I don't think any approach purely based on record counts has any chance to
work well. You could combine it with something else, e.g. with always waking
up in XLogPageRead() and WaitForWALToBecomeAvailable(). But then you end up
with something relatively complicated again.

Greetings,

Andres Freund