On Mon, Nov 6, 2023 at 7:01 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Friday, November 3, 2023 7:32 PM Amit Kapila <amit.kapil...@gmail.com> > > > > 5. > > @@ -228,6 +230,28 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo > > fcinfo, bool confirm, bool bin > > NameStr(MyReplicationSlot->data.plugin), > > format_procedure(fcinfo->flinfo->fn_oid)))); > > .. > > + if (XLogRecPtrIsInvalid(upto_lsn)) > > + wal_to_wait = end_of_wal; > > + else > > + wal_to_wait = Min(upto_lsn, end_of_wal); > > + > > + /* Initialize standby_slot_names_list */ SlotSyncInitConfig(); > > + > > + /* > > + * Wait for specified streaming replication standby servers (if any) > > + * to confirm receipt of WAL upto wal_to_wait. > > + */ > > + WalSndWaitForStandbyConfirmation(wal_to_wait); > > + > > + /* > > + * The memory context used to allocate standby_slot_names_list will be > > + * freed at the end of this call. So free and nullify the list in > > + * order to avoid usage of freed list in the next call to this > > + * function. > > + */ > > + SlotSyncFreeConfig(); > > > > What if there is an error in WalSndWaitForStandbyConfirmation() before > > calling > > SlotSyncFreeConfig()? I think the problem you are trying to avoid by > > freeing it > > here can occur. I think it is better to do this in a logical decoding > > context and > > free the list along with it as we are doing in commit c7256e6564(see PG15). > > I will analyze more about this case and update in next version. >
Okay, thanks for considering it. > > Also, > > it is better to allocate this list somewhere in > > WalSndWaitForStandbyConfirmation(), probably in WalSndGetStandbySlots, > > that will make the code look neat and also avoid allocating this list when > > failover is not enabled for the slot. > > Changed as suggested. > After doing this, do we need to call SlotSyncInitConfig() from other places as below? + SlotSyncInitConfig(); + WalSndGetStandbySlots(&standby_slot_cpy, false); Can we entirely get rid of calling SlotSyncInitConfig() from all places except WalSndGetStandbySlots()? Also, after that or otherwise, the comments atop also need modification. -- With Regards, Amit Kapila.
