MIME-Version: 1.0
References: <CAMPB6wfe4zLjJL8jiZV5kjjpwBM2=rTRme0UCL7Ra4L8MTVdOg@mail.gmail.com>
 <b9666b93-093b-4c9f-a385-772d6ab5688c@app.fastmail.com> <CAMPB6we7+97L72Ru0=WxMDi24xMbZgr2B8Nwoo5i=r=UNuG_gQ@mail.gmail.com>
 <a4ee2198-5309-4b6f-90ef-c08b45a2f572@app.fastmail.com> <CAA4eK1JfPPFTmz7mUk26zPH8+qH9UBpkquxw75x7Ngx_D_6XXQ@mail.gmail.com>
 <CAMPB6wfgvWjSvKNPoJkRqaL46geRDoL++Pt_3Czc2QNAdpVQHw@mail.gmail.com>
 <CAA4eK1JC6yB6q52qEZ5dLNWRUEZoO-aa_XKBZ3_mcb=V2z7zug@mail.gmail.com>
 <CAMPB6weUqU6P2w5VUGVSLKWcvU1AQHmW+7O9qc9yD4CB5kEYVA@mail.gmail.com>
 <CAA4eK1Lm_W5j3DPj6PDSTyodGu87QgxpNwwsi-wVR0+B1FSOoA@mail.gmail.com>
 <CAMPB6wckvkKrXVPH5j8Ske2cVedkb-TRLdnOb5e74zYM1CynGw@mail.gmail.com>
 <CAA4eK1+NDjprcKvr0p2GDMTCs9yxFCY41bOd+6avqAm2n+TXdQ@mail.gmail.com>
 <CAMPB6wdc10tc7gpVXG75r51M41zVSabip9Lz7hssWEtyhecWww@mail.gmail.com>
 <OSCPR01MB14966201F1DCB853145912FF1F53DA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAMPB6wcOWBURHB1igRgCjD3geAemdoATfkKByMwrMM1TgMN64w@mail.gmail.com>
 <OSCPR01MB14966BF4CA9B767C259DBD9CDF53EA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAMPB6wdtKZLEeZ7UW=DYmXWv8Y=uVGrDqXTMhT19Z4VTzo3cfg@mail.gmail.com>
 <CAA4eK1LHVd8wQzauWgeEV436L7btrCfujPH1sR196sY_Mp8zYA@mail.gmail.com>
 <CAMPB6wdPtjbR93oB1XJtYkRtTR64BJG4o5a+0DSSez=puuyuGA@mail.gmail.com>
 <OSCPR01MB14966FC456D053AB00A2EB278F514A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAA4eK1++mHd-SsHyJd2ZB26F7kCz--LbtjQLuQ0h3z9mcYK-AQ@mail.gmail.com>
 <OS7PR01MB149681B14B2432A9CEA7A3586F517A@OS7PR01MB14968.jpnprd01.prod.outlook.com>
 <OSCPR01MB14966F65DE462D0A479B8ADD6F516A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAA4eK1LeyzuiRPZB+o7mO0pB6_=tpkjoum5Hj+t1SYydS4K2kQ@mail.gmail.com>
 <OSCPR01MB14966B68C2148C1BC462AA906F516A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <TY4PR01MB169077EE72ABE9E55BAF162D494B5A@TY4PR01MB16907.jpnprd01.prod.outlook.com>
 <CAJpy0uAcd4BuRGJT0_Xojjtn7w6uiWhAY3Xv=C2GqXoUxeOrYw@mail.gmail.com>
 <CAJpy0uCH=z+x8Rb9U+DCE8=4=7UiG6P_yXoPxGLfp_Nuz5jPfw@mail.gmail.com>
 <CAA4eK1KkqQD88Td53v6Z=adGSMLR7wN543WhqccOW17ykt-QDg@mail.gmail.com> <TY7PR01MB14554CAF66B62811DA167CF0BF582A@TY7PR01MB14554.jpnprd01.prod.outlook.com>
In-Reply-To: <TY7PR01MB14554CAF66B62811DA167CF0BF582A@TY7PR01MB14554.jpnprd01.prod.outlook.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Mon, 12 Jan 2026 11:31:02 +0530
Message-ID: <CAJpy0uB19aKEgVgh8gwzj87NUyDgOf01boa-6xJZK+nhb=3W4g@mail.gmail.com>
Subject: Re: [Patch] add new parameter to pg_replication_origin_session_setup
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, 
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Doruk Yilmaz <doruk@mixrank.com>, 
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAJpy0uB19aKEgVgh8gwzj87NUyDgOf01boa-6xJZK%2Bnhb%3D3W4g%40mail.gmail.com>
Precedence: bulk

On Fri, Jan 9, 2026 at 4:58=E2=80=AFPM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Amit, Shveta,
>
> > > >
> > > > Thanks Hou-San and Kuroda-San.
> > > >
> > > > What should be the expected behavior when Session1 resets the origi=
n
> > > > (changing acquired_pid from its own PID to 0), while Session2 is
> > > > already connected to the origin and Session3 also attempts to reuse
> > > > the same origin?
> > > >
> > > > Currently it asserts:
> > > >
> > > > Session1:
> > > > select pg_replication_origin_create('origin');
> > > > SELECT pg_replication_origin_session_setup('origin');
> > > >
> > > > Session2:
> > > > SELECT pg_replication_origin_session_setup('origin',48028);
> > > >
> > > > Session1:
> > > > SELECT pg_replication_origin_session_reset();
> > > >
> > > > Session3:
> > > > SELECT pg_replication_origin_session_setup('origin');
> > > > This asserts at:
> > > > TRAP: failed Assert("session_replication_state->refcount =3D=3D 0")=
, File:
> > > > "origin.c", Line: 1231, PID: 48037
> > > >
>
> FYI, this happened because v1 assumed refcount was 0 if acquired_by was 0=
.
> But your proposed scenario met it.
>
> > > I checked the behavior on HEAD. Session3 is able to set up the origin
> > > and sets its own PID in acquired_pid. But it is unclear to me which
> > > PID should be recorded in acquired_pid - Session2=E2=80=99s PID, sinc=
e it set
> > > up the origin earlier, or Session3=E2=80=99s PID. Or does this even m=
ake any
> > > difference?
>
> To clarify, I think the behavior on HEAD is not correct. The backend shou=
ld
> acquire the active origin if it expressly specifies the PID. Otherwise, u=
sers
> may acquire unintentionally and advance it.

I agree.

>
> > Can we address these problems by prohibiting leader worker to reset
> > when pa workers are still associated with the origin? The way for
> > leader to know if pa workers are associated with origin is by checking
> > following condition: acquired_by =3D=3D MyProcpid AND refcount > 1.
>
> I think it's okay. IIUC, the idea is to avoid that active origin has inva=
lid
> acquired_by attribute. The replication origin was extended to support par=
allel
> apply of logical replication, and it is reasonable to force the same appr=
oach.
> Attached 0001 implemented that.
>
> One concern with the implementation is that acquired_by can be zero if th=
e process
> exits without releasing the origin; this can happen if the first acquired=
 process
> exits while the second is still using it.
> Regarding our logical replication, it won't be problematic because the le=
ader
> worker ensures all parallel workers finish before it exits.
>
> To address the issue, I propose that another process should not be able t=
o
> acquire the origin if the acquired_by of the active origin is 0. The prob=
lem
> should be resolved within the SQL interface, since it occurs there.
>

+1.

Please find a few comments:

1)
+ /*
+ * The replication origin cannot be reset if the replication origin is
+ * firstly acquired by this backend and other processes are actively using
+ * now. This can cause acquired_by to be zero and active replication origi=
n
+ * might be dropped.
+ */
+ if (session_replication_state->acquired_by =3D=3D MyProcPid &&
+ session_replication_state->refcount > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg_plural("another process is acquiring the replication origin",
+    "other processes are acquiring the replication origin",
+

Since user is not aware of internal acquired_by logic, the error might
not make much sense to him as to why one session is able to reset
while another is not. Shall we make it:

ERROR:  cannot reset replication origin "origin_name" while it is
still shared by other processes
DETAIL:  the current session is the first process for this replication
origin, and other processes are sharing it.
HINT:  ensure this replication origin is reset in all other processes first=
.

2)
When the first session leaves, while the second session is still using
origin, the third session is able to drop the origin which is not
right.
I think replorigin_state_clear() needs a change.
'if (state->acquired_by !=3D 0)' check should be replaced by 'if
(state->refcount > 0)'

Patch 001 had correct changes in replorigin_state_clear(), IMO we
still need those

3)
When first session leaves, while second session is still using origin,
now correctly third session is not able to join it. It gives error:
postgres=3D# SELECT pg_replication_origin_session_setup('origin');
ERROR: replication origin with ID 1 is already active for another process

Error is not very informative provided the fact that now sharing is
allowed. Shall it be:

ERROR:  replication origin "origin_name" cannot be acquired while it
is still in use
DETAIL:  the process that first acquired this origin exited without
releasing it.
HINT:  wait until all processes sharing this origin have released it.
Or
HINT:  ensure this replication origin is reset in all other processes first=
.

4)
+ /* Number of backend that is currently using this origin. */

Number of backend that is --> Count of backends that are

thanks
Shveta