Re: Logical Replication of sequences
shveta malik <shveta.malik@gmail.com>
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Doc: Add documentation for sequence synchronization.
- 55cefadde874 19 (unreleased) landed
-
Remove unused assignment in CREATE PUBLICATION grammar.
- bfb7419b0bbe 19 (unreleased) landed
-
Add seq_sync_error_count to subscription statistics.
- f6a4c498dcf6 19 (unreleased) landed
-
Fix few issues in commit 5509055d69.
- 5a4eba558aa7 19 (unreleased) landed
-
Add sequence synchronization for logical replication.
- 5509055d6956 19 (unreleased) landed
-
Add worker type argument to logical replication worker functions.
- 3e8e05596a02 19 (unreleased) landed
-
Introduce "REFRESH SEQUENCES" for subscriptions.
- f0b3573c3aac 19 (unreleased) landed
-
Refactor logical worker synchronization code into a separate file.
- 41c674d2e31e 19 (unreleased) landed
-
Standardize use of REFRESH PUBLICATION in code and messages.
- 2436b8c047ff 19 (unreleased) landed
-
Add "ALL SEQUENCES" support to publications.
- 96b378497346 19 (unreleased) landed
-
Expose sequence page LSN via pg_get_sequence_data.
- b93172ca59f4 19 (unreleased) landed
-
Resume conflict-relevant data retention automatically.
- 0d48d393d465 19 (unreleased) cited
-
Post-commit review fixes for 228c370868.
- 1f7e9ba3ac4e 19 (unreleased) cited
-
Generate GUC tables from .dat file
- 63599896545c 19 (unreleased) cited
Please find few initial comments for 002:
1)
Patch commit msg says:
"This patch adds support for a new SQL command:
ALTER SUBSCRIPTION ... REFRESH SEQUENCES
This command update the sequence entries present in the
pg_subscription_rel catalog table with the INIT state to trigger
resynchronization."
But AlterSubscription_refresh_seq actually updates the state to DATASYNC.
2)
CheckSubscriptionRelkind()
+ /*
+ * Allow RELKIND_RELATION and RELKIND_PARTITIONED_TABLE to be treated
+ * interchangeably, but ensure that sequences (RELKIND_SEQUENCE) match
+ * exactly on both publisher and subscriber.
+ */
+ if ((relkind == RELKIND_SEQUENCE && pubrelkind != RELKIND_SEQUENCE) ||
+ ((relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE) &&
+ !(pubrelkind == RELKIND_RELATION || pubrelkind == RELKIND_PARTITIONED_TABLE)))
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("relation \"%s.%s\" type mismatch: source \"%s\", target \"%s\"",
+ nspname, relname,
+ pubrelkind == RELKIND_SEQUENCE ? "sequence" : "table",
+ relkind == RELKIND_SEQUENCE ? "sequence" : "table"));
Shall we simplify the check as:
if ((relkind == RELKIND_SEQUENCE && pubrelkind != RELKIND_SEQUENCE) ||
(relkind != RELKIND_SEQUENCE && pubrelkind == RELKIND_SEQUENCE))
3)
CreateSubscription()
+ relations = fetch_relation_list(wrconn, publications);
Can we please rename 'relations' to 'pubrels', as the latter gives
more clarity and is in consistency with AlterSubscription_refresh().
4)
- CheckSubscriptionRelkind(get_rel_relkind(relid),
+ CheckSubscriptionRelkind(relkind, relinfo->relkind,
rv->schemaname, rv->relname);
We are passing 2 relkinds now to CheckSubscriptionRelkind() but it is
difficult to understand which is which. Can we please rename relinfo
as pubrelinfo so that we get clarity. This is in both
CreateSubscription and AlterSubscription_refresh/
5)
CheckSubscriptionRelkind
+ if (pubrelkind == '\0')
+ return;
Do you think, we shall write a comment in the function header that the
caller who wants to verify only the supported type should pass
pubrelkind as '\0'?
6)
Should we update doc of pg_subscription_rel where it says this:
This catalog only contains tables known to the subscription after
running either CREATE SUBSCRIPTION or ALTER SUBSCRIPTION ... REFRESH
PUBLICATION.
thanks
Shveta