Horton

Re: Logical Replication of sequences

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Hou, Zhijie/侯 志杰 <houzj.fnst@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2024-08-15T06:27:01Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Add documentation for sequence synchronization.

  2. Remove unused assignment in CREATE PUBLICATION grammar.

  3. Add seq_sync_error_count to subscription statistics.

  4. Fix few issues in commit 5509055d69.

  5. Add sequence synchronization for logical replication.

  6. Add worker type argument to logical replication worker functions.

  7. Introduce "REFRESH SEQUENCES" for subscriptions.

  8. Refactor logical worker synchronization code into a separate file.

  9. Standardize use of REFRESH PUBLICATION in code and messages.

  10. Add "ALL SEQUENCES" support to publications.

  11. Expose sequence page LSN via pg_get_sequence_data.

  12. Resume conflict-relevant data retention automatically.

  13. Post-commit review fixes for 228c370868.

  14. Generate GUC tables from .dat file

Attachments

Hi Vignesh, I have reviewed your latest patchset:

v20240814-0001. No comments
v20240814-0002. No comments
v20240814-0003. No comments
v20240814-0004. See below
v20240814-0005. No comments

//////

v20240814-0004.

======
src/backend/commands/subscriptioncmds.c

CreateSubscription:
nit - XXX comments

AlterSubscription_refresh:
nit - unnecessary parens in ereport

AlterSubscription:
nit - unnecessary parens in ereport

fetch_sequence_list:
nit - unnecessary parens in ereport

======
.../replication/logical/sequencesync.c

1. fetch_remote_sequence_data

+ * Returns:
+ * - TRUE if there are discrepancies between the sequence parameters in
+ *   the publisher and subscriber.
+ * - FALSE if the parameters match.
+ */
+static bool
+fetch_remote_sequence_data(WalReceiverConn *conn, Oid relid, Oid remoteid,
+    char *nspname, char *relname, int64 *log_cnt,
+    bool *is_called, XLogRecPtr *page_lsn,
+    int64 *last_value)

IMO it is more natural to return TRUE for good results and FALSE for
bad ones. (FYI, I have implemented this reversal in the nitpicks
attachment).

~

nit - swapped columns seqmin and seqmax in the SQL to fetch them in
the natural order
nit - unnecessary parens in ereport

~~~

copy_sequence:
nit - update function comment to document the output parameter
nit - Assert that *sequence_mismatch is false on entry to this function
nit - tweak wrapping and add \n in the SQL
nit - unnecessary parens in ereport

report_sequence_mismatch:
nit - modify function comment
nit - function name changed
/report_sequence_mismatch/report_mismatched_sequences/ (now plural
(and more like the other one)

append_mismatched_sequences:
nit - param name /rel/seqrel/

~~~

2. LogicalRepSyncSequences:
+ Relation sequence_rel;
+ XLogRecPtr sequence_lsn;
+ bool sequence_mismatch;

The 'sequence_mismatch' variable must be initialized false, otherwise
we cannot trust it gets assigned.

~

LogicalRepSyncSequences:
nit - unnecessary parens in ereport
nit - move the for-loop variable declaration
nit - remove a blank line

process_syncing_sequences_for_apply:
nit - variable declaration indent

======
Kind Regards,
Peter Smith.
Fujitsu Australia