Re: Synchronizing slots from primary to standby

On Wed, Jan 17, 2024 at 4:00 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> PFA v63.
>

1.
+ /* User created slot with the same name exists, raise ERROR. */
+ if (!synced)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("exiting from slot synchronization on receiving"
+    " the failover slot \"%s\" from the primary server",
+    remote_slot->name),
+ errdetail("A user-created slot with the same name already"
+   " exists on the standby."));

I think here primary error message should contain the reason for
failure. Something like: "exiting from slot synchronization because
same name slot already exists on standby" then we can add more details
in errdetail.

2.
+synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
{
...
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+ xmin_horizon = GetOldestSafeDecodingTransactionId(true);
+ SpinLockAcquire(&slot->mutex);
+ slot->data.catalog_xmin = xmin_horizon;
+ SpinLockRelease(&slot->mutex);
...
}

Here, why slot->effective_catalog_xmin is not updated? The same is
required by a later call to ReplicationSlotsComputeRequiredXmin(). I
see that the prior version v60-0002 has the corresponding change but
it is missing in the latest version. Any reason?

3.
+ * Return true either if the slot is marked as RS_PERSISTENT (sync-ready) or
+ * is synced periodically (if it was already sync-ready). Return false
+ * otherwise.
+ */
+static bool
+update_and_persist_slot(RemoteSlot *remote_slot)

The second part of the above comment (or is synced periodically (if it
was already sync-ready)) is not clear to me. Does it intend to
describe the case when we try to update the already created temp slot
in the last call. If so, that is not very clear because periodically
sounds like it can be due to repeated sync for sync-ready slot.

4.
+update_and_persist_slot(RemoteSlot *remote_slot)
{
...
+ (void) local_slot_update(remote_slot);
...
}

Can we write a comment to state the reason why we don't care about the
return value here?

--
With Regards,
Amit Kapila.