On Fri, Jan 5, 2024 at 8:59 AM shveta malik <shveta.ma...@gmail.com> wrote: > > On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot > <bertranddrouvot...@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.
