Re: Synchronizing slots from primary to standby

On Tue, Feb 13, 2024 at 9:38 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Here is the V85_2 patch set that added the test and fixed one typo,
> there are no other code changes.
>

Few comments on the latest changes:
==============================
1.
+# Confirm that the invalidated slot has been dropped.
+$standby1->wait_for_log(qr/dropped replication slot "lsub1_slot" of dbid 5/,
+ $log_offset);

Is it okay to hardcode dbid 5? I am a bit worried that it can lead to
instability in the test.

2.
+check_primary_info(WalReceiverConn *wrconn, int elevel)
+{
..
+ bool primary_info_valid;

I don't think for 0001, we need an elevel as an argument, so let's
remove it. Additionally, can we change the variable name
primary_info_valid to primary_slot_valid? Also, can we change the
function name to validate_remote_info() as the remote can be both
primary or standby?

3.
+SyncReplicationSlots(WalReceiverConn *wrconn)
+{
+ PG_ENSURE_ERROR_CLEANUP(slotsync_failure_callback, PointerGetDatum(wrconn));
+ {
+ check_primary_info(wrconn, ERROR);
+
+ synchronize_slots(wrconn);
+ }
+ PG_END_ENSURE_ERROR_CLEANUP(slotsync_failure_callback,
PointerGetDatum(wrconn));
+
+ walrcv_disconnect(wrconn);

It is better to disconnect in the caller where we have made the connection.

--
With Regards,
Amit Kapila.