Re: [Patch] add new parameter to pg_replication_origin_session_setup

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.
> 
> > * 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.
> 
> I updated/rewrote the patch to reflect these suggestions.
> It now has the same DEFAULT 0 style used in pg_create_logical_replication_slot.
> I also updated the documentation.

[after a long hiatus...]

I tested your patch again and it does what is advertised. I changed your patch
a bit. The main change was the documentation. You didn't explain what this new
parameter is for. I tried to explain but don't want to add lots of details.
(There is a section that explain how parallel apply processes work behind the
scenes.) I also renamed it from acquired_by to pid to be more descriptive. I
fixed some white space issues too. I noticed that there are no tests. This
doesn't appear to be a shortcoming from this patch but we need to cover some of
these replication functions with an additional test file in another patch.
Finally, I wrote a commit message and it is RfC.

session 1:

postgres=# select * from pg_replication_origin;
roident | roname 
---------+--------
(0 rows)

postgres=# SELECT pg_backend_pid();
pg_backend_pid 
----------------
         260732
(1 row)

postgres=# SELECT pg_replication_origin_create('test');
pg_replication_origin_create 
------------------------------
                            1
(1 row)

postgres=# SELECT pg_replication_origin_session_setup('test', 0);
pg_replication_origin_session_setup 
-------------------------------------
 
(1 row)

postgres=# select * from pg_replication_origin;
roident | roname 
---------+--------
       1 | test
(1 row)

session 2:

postgres=# SELECT pg_replication_origin_session_setup('test', 260732);
pg_replication_origin_session_setup 
-------------------------------------
 
(1 row)

session 3:

postgres=# SELECT pg_replication_origin_session_setup('test', 12345);
ERROR:  could not find replication state slot for replication origin with OID 1 which was acquired by 12345


--
Euler Taveira
EDB   https://www.enterprisedb.com/