Here are some review comments for patch v25-0002
(additional to v25-0002 review comments [1])
======
src/backend/catalog/system_views.sql
1.
@@ -1003,7 +1003,8 @@ CREATE VIEW pg_replication_slots AS
L.safe_wal_size,
L.two_phase,
L.conflicting,
- L.failover
+ L.failover,
+ L.synced_slot
FROM pg_get_replication_slots() AS L
LEFT JOIN pg_database D ON (L.datoid = D.oid);
AFAICT the patch is missing PG DOCS descriptions for these new view attributes.
======
src/backend/replication/logical/launcher.c
2. slotsync_remove_obsolete_dbs
+
+ /*
+ * TODO: Take care of of removal of old 'synced' slots for the dbs which
+ * are no longer eligible for slot-sync.
+ */
typo: "of of"
~~~
3.
+ /*
+ * Make sure that concerned WAL is received before syncing slot to target
+ * lsn received from the primary.
+ *
+ * This check should never pass as on the primary, we have waited for
+ * standby's confirmation before updating the logical slot. But to take
+ * care of any bug in that flow, we should retain this check.
+ */
+ if (remote_slot->confirmed_lsn > WalRcv->latestWalEnd)
+ {
+ ereport(LOG,
+ errmsg_internal("skipping sync of slot \"%s\" as the received slot-sync "
+ "lsn %X/%X is ahead of the standby position %X/%X",
+ remote_slot->name,
+ LSN_FORMAT_ARGS(remote_slot->confirmed_lsn),
+ LSN_FORMAT_ARGS(WalRcv->latestWalEnd)));
+ return;
+ }
Would elog be better here than using ereport(LOG, errmsg_internal...);
IIUC it does the same thing?
======
[1]
https://www.postgresql.org/message-id/CAHut%2BPspseC03Fhsi%3DOqOtksagspE%2B0MVOhrhhUb64cc_4SE1w%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
