On Fri, Jan 5, 2024 at 8:59 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > 4 ===
> >
> > Looking closer, the only place where walrcv_connect() is called with replication
> > set to false and logical set to false is in ReplSlotSyncWorkerMain().
> >
> > That does make sense, but what do you think about creating dedicated libpqslotsyncwrkr_connect
> > and slotsyncwrkr_connect (instead of using the libpqrcv_connect / walrcv_connect ones)?
> >
> > That way we could make use of slotsyncwrkr_connect() in ReplSlotSyncWorkerMain()
> > as I think it's confusing to use "rcv" functions while the process using them is
> > not of backend type walreceiver.
> >
> > I'm not sure that worth the extra complexity though, what do you think?
>
> I gave it a thought earlier, but then I was not sure even if I create
> a new function w/o "rcv" in it then where should it be placed as the
> existing file name itself is libpq'walreceiver'.c. Shall we be
> creating a new file then? But it does not seem good to create a new
> setup (new file, function pointers other stuff) around 1 function.
> And thus reusing the same function with 'replication' (new arg) felt
> like a better choice than other options. If in future, there is any
> other module trying to do the same, then it can use current
> walrcv_connect() with rep=false. If I make it specific to slot-sync
> worker, then it will not be reusable by other modules (if needed).
>
I agree that the benefit of creating a new API is not very clear. How
about adjusting the description in the file header of
libpqwalreceiver.c. I think apart from walreceiver, it is now also
used by logical replication workers and with this patch by the
slotsync worker as well.
--
With Regards,
Amit Kapila.
