public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: shveta malik <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Doruk Yilmaz <[email protected]>
Subject: Re: [Patch] add new parameter to pg_replication_origin_session_setup
Date: Wed, 25 Mar 2026 13:51:14 +0530
Message-ID: <CAA4eK1JWM_GxnDn8T0+B3+dEqzACkUj7rvfy_B_ukq5myvqxGg@mail.gmail.com> (raw)
In-Reply-To: <CAJpy0uBm7m5AFgg6xgWB6msGrivusoDe7PKg9XKHYm_JYQnegg@mail.gmail.com>
References: <CAJpy0uB19aKEgVgh8gwzj87NUyDgOf01boa-6xJZK+nhb=3W4g@mail.gmail.com>
<CAA4eK1+h4mOvRqRaGfUtSgZuBhzWWmrBcY3jQ4DDV=cEJ4dwnQ@mail.gmail.com>
<TY7PR01MB145543A74547443E49E4993E7F58EA@TY7PR01MB14554.jpnprd01.prod.outlook.com>
<CAJpy0uD6D294d=Hq4oROmtKAew5DfKERNxs=DsAwUFBFF2kERg@mail.gmail.com>
<TY7PR01MB14554FB7D02601425DE3752DEF58FA@TY7PR01MB14554.jpnprd01.prod.outlook.com>
<CAA4eK1JvYROk5usAX0Uy=00VACB=w1rvUcyoKvRuke5EMfmi3Q@mail.gmail.com>
<[email protected]>
<CAJpy0uD5T692uyaRr0GKqd1eLHW55A0=MH-V8tRPicTxDqMu=g@mail.gmail.com>
<CAA4eK1LN+T1pLfGBxDW-gps5h-YX6FAjRwkKaV2wwQDDVdi6fw@mail.gmail.com>
<CAJpy0uBm7m5AFgg6xgWB6msGrivusoDe7PKg9XKHYm_JYQnegg@mail.gmail.com>
On Wed, Feb 11, 2026 at 5:09 PM shveta malik <[email protected]> wrote:
>
> On Wed, Feb 11, 2026 at 3:41 PM Amit Kapila <[email protected]> wrote:
> >
> >
> > I see your point but one advantage with the proposed code change is
> > that it started to appear that we can extend this part of code easily
> > in the future as it separates most of the handling related to when a
> > user has given acquired_by parameter's value as zero and non-zero.
>
> Okay, yes. So I am okay with it. The slight change I suggested (if to
> else-if) and a comment will make it more clean.
>
I have tried to address both your suggestions in the attached. See, if
this looks okay to you now?
--
With Regards,
Amit Kapila.
Attachments:
[application/octet-stream] v2-0001-Simplify-replorigin_session_setup.patch (4.3K, 2-v2-0001-Simplify-replorigin_session_setup.patch)
download | inline diff:
From 494b31e79dce2a08fb56cee2b6af521242be043f Mon Sep 17 00:00:00 2001
From: Amit Kapila <[email protected]>
Date: Wed, 25 Mar 2026 13:42:52 +0530
Subject: [PATCH v3] Simplify replorigin_session_setup()
---
src/backend/replication/logical/origin.c | 78 ++++++++++++++----------
1 file changed, 46 insertions(+), 32 deletions(-)
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 26afd8f0af9..26c3725aa68 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -1186,55 +1186,69 @@ replorigin_session_setup(ReplOriginId node, int acquired_by)
if (curstate->roident != node)
continue;
- else if (curstate->acquired_by != 0 && acquired_by == 0)
+ if (acquired_by == 0)
{
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_IN_USE),
- errmsg("replication origin with ID %d is already active for PID %d",
- curstate->roident, curstate->acquired_by)));
+ /* With acquired_by == 0, we need the origin to be free */
+ if (curstate->acquired_by != 0)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("replication origin with ID %d is already active for PID %d",
+ curstate->roident, curstate->acquired_by)));
+ }
+ else if (curstate->refcount > 0)
+ {
+ /*
+ * The origin is in use, but PID is not recorded. This can
+ * happen if the process that originally acquired the origin
+ * exited without releasing it. To ensure correctness, other
+ * processes cannot acquire the origin until all processes
+ * currently using it have released it.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("replication origin with ID %d is already active in another process",
+ curstate->roident)));
+ }
}
-
- else if (curstate->acquired_by != acquired_by)
+ else
{
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_IN_USE),
- errmsg("could not find replication state slot for replication origin with OID %u which was acquired by %d",
- node, acquired_by)));
+ /*
+ * With acquired_by != 0, we need the origin to be active by the
+ * given PID
+ */
+ if (curstate->acquired_by != acquired_by)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("replication origin with ID %d is not active for PID %d",
+ curstate->roident, acquired_by)));
+ /*
+ * Here, it is okay to have refcount > 0 as more than one process
+ * can safely re-use the origin.
+ */
}
- /*
- * The origin is in use, but PID is not recorded. This can happen if
- * the process that originally acquired the origin exited without
- * releasing it. To ensure correctness, other processes cannot acquire
- * the origin until all processes currently using it have released it.
- */
- else if (curstate->acquired_by == 0 && curstate->refcount > 0)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_IN_USE),
- errmsg("replication origin with ID %d is already active in another process",
- curstate->roident)));
-
/* ok, found slot */
session_replication_state = curstate;
break;
}
-
- if (session_replication_state == NULL && free_slot == -1)
- ereport(ERROR,
- (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
- errmsg("could not find free replication state slot for replication origin with ID %d",
- node),
- errhint("Increase \"max_active_replication_origins\" and try again.")));
- else if (session_replication_state == NULL)
+ if (session_replication_state == NULL)
{
- if (acquired_by)
+ if (acquired_by != 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot use PID %d for inactive replication origin with ID %d",
acquired_by, node)));
/* initialize new slot */
+ if (free_slot == -1)
+ ereport(ERROR,
+ (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("could not find free replication state slot for replication origin with ID %d",
+ node),
+ errhint("Increase \"max_active_replication_origins\" and try again.")));
+
session_replication_state = &replication_states[free_slot];
Assert(!XLogRecPtrIsValid(session_replication_state->remote_lsn));
Assert(!XLogRecPtrIsValid(session_replication_state->local_lsn));
--
2.52.0.windows.1
view thread (30+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [Patch] add new parameter to pg_replication_origin_session_setup
In-Reply-To: <CAA4eK1JWM_GxnDn8T0+B3+dEqzACkUj7rvfy_B_ukq5myvqxGg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox