Horton

Re: Logical Replication of sequences

Chao Li <li.evan.chao@gmail.com>

View thread
From: Chao Li <li.evan.chao@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, vignesh C <vignesh21@gmail.com>, Peter Smith <smithpb2250@gmail.com>, shveta malik <shveta.malik@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2025-10-17T09:34:16Z
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

I just reviewed 0001 and got a few comments wrt code comments. I may find some time to review 0002 and 0003 next week.

> On Oct 16, 2025, at 19:23, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
> 
> <v20251016-0003-Documentation-for-sequence-synchronization.patch><v20251016-0002-New-worker-for-sequence-synchronization-du.patch><v20251016-0001-Introduce-REFRESH-SEQUENCES-for-subscripti.patch>

1 - 0001 - pg_subscription.c
```
+		/*
+		 * Skip sequence tuples. If even a single table tuple exists then the
+		 * subscription has tables.
+		 */
+		if (get_rel_relkind(subrel->srrelid) == RELKIND_RELATION ||
+			get_rel_relkind(subrel->srrelid) == RELKIND_PARTITIONED_TABLE)
+		{
+			has_subrels = true;
+			break;
+		}
```

The comment "If even a single table tuple exists then the subscription has tables” sounds redundant. I know it’s inherited from the old code, but now, with the “break” you newly added, the code logic is simple and clear, so I think the comment is no longer needed.

2 - 0001  - pg_subscription.c
```
@@ -542,12 +560,21 @@ HasSubscriptionTables(Oid subid)
+ * get_tables: get relations for tables of the subscription.
+ *
+ * get_sequences: get relations for sequences of the subscription.
+ *
+ * not_ready:
+ * If getting tables and not_ready is false, retrieve all tables;
+ * otherwise, retrieve only tables that have not reached the READY state.
+ * If getting sequences and not_ready is false, retrieve all sequences;
+ * otherwise, retrieve only sequences that have not reached the READY state.
+ *
```

This function comment sounds a bit verbose and repetitive. Suggested revision:
```
* get_tables: if true, include tables in the returned list.
 * get_sequences: if true, include sequences in the returned list.
 * not_ready: if true, include only objects that have not reached the READY state;
 *            if false, include all objects of the requested type(s).
```

3 - 0001 - subscriptioncmds.c
```
+			 * Currently, a replication slot is created for all subscriptions,
+			 * including those for empty or sequence-only publications. While
+			 * this is unnecessary, optimizing this behavior would require
+			 * additional handling to ensure the apply worker operates
+			 * smoothly without acquiring a slot on the publisher, thus adding
+			 * complexity to the apply worker. Given that such subscriptions
+			 * are infrequent, it doesn't seem to be worth doing anything
+			 * about it.
```

Minor tweaks:
* "optimizing this behavior” -> “optimizing it”
* “doing anything about it” -> “addressing it"

4 - 0001 - subscriptioncmds.c
```
 * 3) For ALTER SUBSCRIPTION ... REFRESH SEQUENCE statements with "copy_data =
* true" and "origin = none":
* - Warn the user that sequence data from another origin might have been
* copied.
```

“Warn the user” -> “Warn users"

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/