Thread: [Patch] add new parameter to pg_replication_origin_session_setup
Hello all,
While working on our internal tools that utilise replication, we
realised that a new parameter was added to the internal C function
corresponding to pg_replication_origin_session_setup.
However this parameter wasn't included in the user-facing API [1].
In 'src/backend/replication/logical/origin.c' at line 1359,
pg_replication_origin_session_setup function calls
replorigin_session_setup(origin, 0);
where currently 0 is assigned to the acquired_by parameter of the
replorigin_session_setup.
I made this patch to the master which adds a way to control this
parameter by adding a new version of the
pg_replication_origin_session_setup function with user facing
parameters 'text int4' in place of the current 'text' while keeping
the existing variant
(ensuring backwards compatibility). Could someone take a look at it?
[1]: https://www.postgresql.org/docs/current/functions-admin.html#PG-REPLICATION-ORIGIN-SESSION-SETUP
---
Thanks for the help,
Doruk Yılmaz
Attachment
On Mon, Aug 12, 2024, at 3:43 PM, Doruk Yilmaz wrote:
Hello all,
Hi!
While working on our internal tools that utilise replication, werealised that a new parameter was added to the internal C functioncorresponding to pg_replication_origin_session_setup.However this parameter wasn't included in the user-facing API [1].
I'm curious about your use case. Is it just because the internal function has a
different signature or your tool is capable of apply logical replication changes
in parallel using the SQL API?
I made this patch to the master which adds a way to control thisparameter by adding a new version of thepg_replication_origin_session_setup function with user facingparameters 'text int4' in place of the current 'text' while keepingthe existing variant(ensuring backwards compatibility). Could someone take a look at it?
I did a quick look at your patch and have a few suggestions.
* no documentation changes. Since the function you are changing has a new
signature, this change should be reflected in the documentation.
* no need for a new internal function. The second parameter (PID) can be
optional and defaults to 0 in this case. See how we changed the
pg_create_logical_replication_slot along the years add some IN parameters like
twophase and failover in the recent versions.
* add a CF entry [1] for this patch so we don't forget it. Another advantage is
that this patch is covered by CI [2][3].
I noticed that the patch needs rebasing, so here is the rebased version. Hopefully it makes to the commitfest. Doruk Yılmaz
Attachment
On Thu, Jan 9, 2025 at 3:26 AM Euler Taveira <euler@eulerto.com> wrote: > > On Thu, Aug 15, 2024, at 5:53 PM, Doruk Yilmaz wrote: > > Hello again, > > On Tue, Aug 13, 2024 at 12:48 AM Euler Taveira <euler@eulerto.com> wrote: > > I'm curious about your use case. Is it just because the internal function has a > > different signature or your tool is capable of apply logical replication changes > > in parallel using the SQL API? > > The latter is correct, it applies logical replication changes in parallel. > Since multiple connections may commit, we need all of them to be able > to advance the replication origin. > To use replication_origin by multiple processes, one must maintain the commit order as we do internally by allowing the leader process to wait for the parallel worker to finish the commit. See comments atop replorigin_session_setup(). Now, we could expose the pid parameter as proposed by the patch after documenting the additional requirements, but I am afraid that users may directly start using the API without following the commit order principle, which can lead to incorrect replication. So, isn't it better to do something to avoid the misuse of this feature before exposing it? -- With Regards, Amit Kapila.