Hi Vignesh.
Some minor review comments for the patches in set v20250514.
======
Patch 0001.
1.1
For function 'pg_sequence_state', the DOCS call the 2nd parameter
'sequence_name', but the pg_proc.dat file calls it 'seq_name'. Should
these be made the same?
////////////////////
Patch 0004.
pg_sequence_state:
4.1
- errmsg("sequence \"%s.%s\" does not exist",
+ errmsg("logical replication sequence \"%s.%s\" does not exist",
Why isn't this change already be done in an early patch when this
function was first implemented?
~~~
copy_sequences:
4.2
+/*
+ * Copy existing data of sequnces from the publisher.
+ *
Typo: "sequnces"
~~~
4.3
+{
+ int total_seq = list_length(remotesequences);
+ int curr_seq = 0;
+
+/*
+ * We batch synchronize multiple sequences per transaction, because the
+ * alternative of synchronizing each sequence individually incurs overhead of
+ * starting and committing transactions repeatedly. On the other hand, we want
+ * to avoid keeping this batch transaction open for extended periods so it is
+ * currently limited to 100 sequences per batch.
+ */
+#define MAX_SEQUENCES_SYNC_PER_BATCH 100
Wrong indent for block comment.
~~~
4.4
+ if (res->status != WALRCV_OK_TUPLES)
+ ereport(ERROR,
+ errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not receive list of sequences information from the
publisher: %s",
+ res->err));
Should this say /sequences information/sequence information/
~~~
4.5
+ ereport(LOG,
+ errmsg("Logical replication sequence synchronization - total
unsynchronized: %d, attempted in this batch: %d; succeeded in this
batch: %d; mismatched in this batch: %d for subscription: \"%s\"",
+ total_seq, batch_seq_count, batch_success_count,
+ batch_mismatch_count, get_subscription_name(subid, false)));
+
This errmsg seems backwards. I think it should be expressed like the
other one immediately above. Also I think the information can be made
shorter -- e.g. no need to say "in this batch" multiple times.
SUGGESTION
"Logical replication sequence synchronization for subscription \"%s\"
- total unsynchronized: %d; batch #%d = %d attempted, %d succeeded, %d
mismatched"
======
Kind Regards,
Peter Smith.
Fujitsu Australia
