Few trivial comments on 001:
patch001:
1)
FetchRelationStates
/* Fetch tables and sequences that are in non-ready state. */
- rstates = GetSubscriptionRelations(MySubscription->oid, true);
+ rstates = GetSubscriptionRelations(MySubscription->oid, true, false,
+ true);
We are passing get_Sequenecs as false, we should update comment to get
rid of 'sequences;
2)
+ /*
+ * XXX The walsender currently does not transmit the relkind of the remote
+ * relation when replicating changes. Since we support replicating only
+ * table changes at present, we default to initializing relkind as
+ * RELKIND_RELATION.
+ */
+ entry->remoterel.relkind = remoterel->relkind
+ ? remoterel->relkind : RELKIND_RELATION;
Shall we also mention that currently this is used or needed only for
CheckSubscriptionRelkind().
~~
Few on 002:
3)
sequencesync.c:
+ * Executing the following command resets all sequences in the subscription to
+ * state DATASYNC, triggering re-synchronization:
+ * ALTER SUBSCRIPTION ... REFRESH SEQUENCES
+ * Sequence state transitions follow this pattern:
+ * INIT / DATASYNC → READY
These comments need correction. We no longer use DATASYNC state for seq.
4)
+ * To avoid creating too many transactions, up to MAX_SEQUENCES_SYNC_PER_BATCH
+ * (100) sequences are synchronized per transaction. The locks on the sequence
We can avoid writing 100 here. Mention of macro is enough.
5)
I had some 250 sequences; dropped one on pub, and ran 'refresh
sequences' on sub, noticed few issues:
a) Seqsync worker logged below but actually did not remove it. It
seems it is some pending log from previous implementation:
LOG: sequences not found on publisher removed from resynchronization:
("public.myseq250")
b) It kept on restarting the seq sync worker very fast. It should have
waited for wal_retrieve_retry_interval (which is 5s in my env) to
attempt restarting, isn't it? Logs:
----
14:27:53.492 IST [72327] LOG: logical replication sequence
synchronization worker for subscription "sub1" has started
14:27:53.499 IST [72327] LOG: logical replication sequence
synchronization for subscription "sub1" - total unsynchronized: 1
14:27:53.500 IST [72327] LOG: logical replication sequence
synchronization for subscription "sub1" - batch #1 = 1 attempted, 0
succeeded, 0 skipped, 0 mismatched, 0 insufficient permission, 1
missing from publisher
14:27:53.500 IST [72327] LOG: sequences not found on publisher
removed from resynchronization: ("public.myseq250")
14:27:53.500 IST [72327] LOG: logical replication sequence
synchronization worker for subscription "sub1" has finished
14:27:53.503 IST [72329] LOG: logical replication sequence
synchronization worker for subscription "sub1" has started
14:27:53.508 IST [72329] LOG: logical replication sequence
synchronization for subscription "sub1" - total unsynchronized: 1
----
c) Should this case give a suggestion/HINT in log to run 'REFERSH PUB'
to correct sequence mapping.
thanks
Shveta
