On Wednesday, November 29, 2023 5:12 PM Zhijie Hou (Fujitsu) 
<houzj.f...@fujitsu.com> wrote:

I was reviewing slotsync worker design and here
are few comments on 0002 patch:

1.

+       if (!WalRcv ||
+               (WalRcv->slotname[0] == '\0') ||
+               XLogRecPtrIsInvalid(WalRcv->latestWalEnd))

I think we'd better take spinlock when accessing these shared memory fields.

2.

        /*
         * The slot sync feature itself is disabled, exit.
         */
        if (!enable_syncslot)
        {
                ereport(LOG,
                                errmsg("exiting slot sync worker as 
enable_syncslot is disabled."));

Can we check the GUC when registering the worker(SlotSyncWorkerRegister),
so that the worker won't be started if enable_syncslot is false.

3. In synchronize_one_slot, do we need to skip the slot sync and drop if the
local slot is a physical one ?

4.

                        *locally_invalidated =
                                (remote_slot->invalidated == RS_INVAL_NONE) &&
                                (local_slot->data.invalidated != RS_INVAL_NONE);

When reading the invalidated flag of local slot, I think we'd better take
spinlock.

Best Regards,
Hou zj

Reply via email to