MIME-Version: 1.0
References: <19488-d7ccfca2bf6b74b0@postgresql.org>
 <CAJTYsWUQBwCOQ=uGnLP7C2ofN6TqUpGeOHan2N99mza1DjybsQ@mail.gmail.com>
 <CAHGQGwG=F8_12nPKxUkYm-uS5QFJp+KF=3X5zM2vKP=0SKAeHw@mail.gmail.com>
 <CAPpHfdt_Fj73xYhKtEc66v7c_ZLaUSFqXfCWhRdp3tMX6ksMjQ@mail.gmail.com>
 <CAHGQGwHEVC1181tW78HuotgbF2jMF_59wzKrtS7hFYn_f9tuew@mail.gmail.com>
In-Reply-To: 
 <CAHGQGwHEVC1181tW78HuotgbF2jMF_59wzKrtS7hFYn_f9tuew@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Wed, 20 May 2026 18:15:44 +0530
Message-ID: 
 <CAJTYsWXDoWuXCbgOYfTiBFTxg6-d+CX8urk0KswQX0Fm4JS+ag@mail.gmail.com>
Subject: Re: BUG #19488: Standby connection fails after dropping on login
 event trigger enabled always
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, kyzevan23@mail.ru,
	pgsql-bugs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="0000000000003a721706523f2d34"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWXDoWuXCbgOYfTiBFTxg6-d%2BCX8urk0KswQX0Fm4JS%2Bag%40mail.gmail.com>
Precedence: bulk

--0000000000003a721706523f2d34
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Wed, 20 May 2026 at 17:35, Fujii Masao <masao.fujii@gmail.com> wrote:

> On Wed, May 20, 2026 at 8:31=E2=80=AFPM Alexander Korotkov <aekorotkov@gm=
ail.com>
> wrote:
> >
> > On Wed, May 20, 2026 at 1:03=E2=80=AFPM Fujii Masao <masao.fujii@gmail.=
com>
> wrote:
> > > On Wed, May 20, 2026 at 1:37=E2=80=AFPM Ayush Tiwari
> > > <ayushtiwari.slg01@gmail.com> wrote:
> > > > Thanks for the report and the precise repro.
> > >
> > > +1
> > >
> > > > Attached patch adds a RecoveryInProgress() check to skip the cleanu=
p
> > > > branch on standbys.
> > >
> > > Thanks for investigating this issue and for the patch!
> > > The patch looks good to me.
> > >
> > > > I think this needs to be backpatched too.
> > >
> > > Yes. Seems this should be backpatched to v17, where login event
> triggers
> > > were introduced.
> >
> > I've added a tap test reproducing the bug.  I'm going to push and
> > backpatch this to v17 if no objections.
>
> +# Wait for the standby to replay the CREATE/DROP catalog state.  At
> +# this point the standby's pg_database.dathasloginevt is still true.
> +$primary->wait_for_replay_catchup($standby);
> +
> +# A new connection to the standby exercises EventTriggerOnLogin()'s
> +# cleanup branch.  With the RecoveryInProgress() guard, that branch is
> +# skipped on the standby and the connection succeeds.  Without it the
> +# session aborts with a FATAL about AccessExclusiveLock.  Probing the
> +# flag itself via safe_psql is what triggers the cleanup path.
> +is( $standby->safe_psql(
> + 'postgres',
> + "SELECT dathasloginevt FROM pg_database WHERE datname =3D 'postgres'"),
> + 't',
> + 'standby accepts connection and reports dangling dathasloginevt');
>
> The test looks unstable to me. wait_for_replay_catchup() may connect to
> the primary to obtain the flush LSN, which could cause dathasloginevt to
> become false before the subsequent safe_psql() call on the standby.
>

I think Fuji-san is right, can we do something like this:

my $drop_lsn =3D $primary->safe_psql(
    'postgres', q{
BEGIN;
DROP EVENT TRIGGER init_session;
DROP FUNCTION init_session();
COMMIT;
SELECT pg_current_wal_lsn();
});

$primary->wait_for_catchup($standby, 'replay', $drop_lsn);

Then the following standby connection should reliably exercise the case
where dathasloginevt is still true on the standby but no login event
trigger remains.

Regards,
Ayush

--0000000000003a721706523f2d34
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, 20 May 2=
026 at 17:35, Fujii Masao &lt;<a href=3D"mailto:masao.fujii@gmail.com">masa=
o.fujii@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote"=
 style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);p=
adding-left:1ex">On Wed, May 20, 2026 at 8:31=E2=80=AFPM Alexander Korotkov=
 &lt;<a href=3D"mailto:aekorotkov@gmail.com" target=3D"_blank">aekorotkov@g=
mail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; On Wed, May 20, 2026 at 1:03=E2=80=AFPM Fujii Masao &lt;<a href=3D"mai=
lto:masao.fujii@gmail.com" target=3D"_blank">masao.fujii@gmail.com</a>&gt; =
wrote:<br>
&gt; &gt; On Wed, May 20, 2026 at 1:37=E2=80=AFPM Ayush Tiwari<br>
&gt; &gt; &lt;<a href=3D"mailto:ayushtiwari.slg01@gmail.com" target=3D"_bla=
nk">ayushtiwari.slg01@gmail.com</a>&gt; wrote:<br>
&gt; &gt; &gt; Thanks for the report and the precise repro.<br>
&gt; &gt;<br>
&gt; &gt; +1<br>
&gt; &gt;<br>
&gt; &gt; &gt; Attached patch adds a RecoveryInProgress() check to skip the=
 cleanup<br>
&gt; &gt; &gt; branch on standbys.<br>
&gt; &gt;<br>
&gt; &gt; Thanks for investigating this issue and for the patch!<br>
&gt; &gt; The patch looks good to me.<br>
&gt; &gt;<br>
&gt; &gt; &gt; I think this needs to be backpatched too.<br>
&gt; &gt;<br>
&gt; &gt; Yes. Seems this should be backpatched to v17, where login event t=
riggers<br>
&gt; &gt; were introduced.<br>
&gt;<br>
&gt; I&#39;ve added a tap test reproducing the bug.=C2=A0 I&#39;m going to =
push and<br>
&gt; backpatch this to v17 if no objections.<br>
<br>
+# Wait for the standby to replay the CREATE/DROP catalog state.=C2=A0 At<b=
r>
+# this point the standby&#39;s pg_database.dathasloginevt is still true.<b=
r>
+$primary-&gt;wait_for_replay_catchup($standby);<br>
+<br>
+# A new connection to the standby exercises EventTriggerOnLogin()&#39;s<br=
>
+# cleanup branch.=C2=A0 With the RecoveryInProgress() guard, that branch i=
s<br>
+# skipped on the standby and the connection succeeds.=C2=A0 Without it the=
<br>
+# session aborts with a FATAL about AccessExclusiveLock.=C2=A0 Probing the=
<br>
+# flag itself via safe_psql is what triggers the cleanup path.<br>
+is( $standby-&gt;safe_psql(<br>
+ &#39;postgres&#39;,<br>
+ &quot;SELECT dathasloginevt FROM pg_database WHERE datname =3D &#39;postg=
res&#39;&quot;),<br>
+ &#39;t&#39;,<br>
+ &#39;standby accepts connection and reports dangling dathasloginevt&#39;)=
;<br>
<br>
The test looks unstable to me. wait_for_replay_catchup() may connect to<br>
the primary to obtain the flush LSN, which could cause dathasloginevt to<br=
>
become false before the subsequent safe_psql() call on the standby.<br></bl=
ockquote><div>=C2=A0</div><div>I think Fuji-san is right, can we do somethi=
ng like this:<br><br>my $drop_lsn =3D $primary-&gt;safe_psql(<br>=C2=A0 =C2=
=A0 &#39;postgres&#39;, q{<br>BEGIN;<br>DROP EVENT TRIGGER init_session;<br=
>DROP FUNCTION init_session();<br>COMMIT;<br>SELECT pg_current_wal_lsn();<b=
r>});<br><br>$primary-&gt;wait_for_catchup($standby, &#39;replay&#39;, $dro=
p_lsn);<br><br>Then the following standby connection should reliably exerci=
se the case<br>where dathasloginevt is still true on the standby but no log=
in event<br>trigger remains.<br><br>Regards,<br>Ayush=C2=A0</div></div></di=
v>

--0000000000003a721706523f2d34--