Re: Logical Replication of sequences

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