Re-range clone sequences to new group id during promotion#8638
Re-range clone sequences to new group id during promotion#8638codeforall wants to merge 3 commits into
Conversation
citus_promote_clone_and_rebalance promotes a physical streaming replica to a new primary with a freshly-allocated group id, but the clone's sequence objects are byte-for-byte copies of the source primary's and keep the old primary's (groupId << 48) value window. The classical activation path re-ranges sequences per group id via AlterSequenceMinMax; the clone path did not, so the promoted clone and the original primary emitted values from the same window, breaking the global-uniqueness invariant for bigserial/bigint nextval(...) defaults and GENERATED ... AS IDENTITY columns on distributed tables. Add DependentSequenceRangeAdjustCommandList() (mirrors IdentitySequenceDependencyCommandList for nextval/serial sequences) and a promotion-path helper AdjustCloneSequenceRangesForNewGroup() that re-ranges the clone's distributed-table sequences to its new group-id window. The helper runs after SyncNodeMetadataToNodes() over the reused metadata connection, so the clone's corrected local group id is visible when AlterSequenceMinMax executes. Reuses the existing citus_internal.adjust_identity_column_seq_settings UDF; no SQL migration or catalog change. Regression coverage included
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8638 +/- ##
==========================================
- Coverage 88.73% 88.72% -0.02%
==========================================
Files 288 288
Lines 64385 64420 +35
Branches 8109 8112 +3
==========================================
+ Hits 57132 57155 +23
- Misses 4908 4918 +10
- Partials 2345 2347 +2 🚀 New features to boost your workflow:
|
| { | ||
| List *ddlCommandList = NIL; | ||
|
|
||
| List *distributedTableList = CitusTableTypeIdList(DISTRIBUTED_TABLE); |
There was a problem hiding this comment.
should probably handle all table types here, not only distributed tables
There was a problem hiding this comment.
and should add a few tests for other table types too, e.g., to show that we re-arrange sequence ranges for reference tables / citus local tables etc.
There was a problem hiding this comment.
Good call! Fixed it
We now select tables via ShouldSyncTableMetadata(), the same predicate the classical add/activate path uses, so reference and Citus local tables are re-ranged too.
Thanks!
| ereport(NOTICE, (errmsg( | ||
| "re-ranged %d sequence(s) on promoted clone %s:%d (ID %d) for new group %d", | ||
| list_length(commandList), freshCloneNode->workerName, | ||
| freshCloneNode->workerPort, freshCloneNode->nodeId, | ||
| freshCloneNode->groupId))); |
There was a problem hiding this comment.
This might produce too much log in a a cluster where there are ten thousands of citus tables, should we maybe make it DEBUG2 or such?
There was a problem hiding this comment.
it's logged just once per promotion (a single line after the loop with a total count), not per table, so it won't grow with the number of tables. The reason I put the NOTICE here is, It matches the other [NOTICE] progress messages in promote clone function. Happy to lower it if you'd still prefer though!
Address review feedback: the clone-promotion sequence re-range only covered distributed tables, but classical add/activate-node re-ranges sequences for every table synced to metadata workers -- reference and Citus local tables included (both support worker-side sequence defaults). AdjustCloneSequenceRangesForNewGroup now iterates AllCitusTableIds() filtered by ShouldSyncTableMetadata(), the same predicate the classical metadata sync uses, so both node-addition paths re-range the same set of sequences. Extend multi_add_node_from_backup_sequences to create distributed, reference and Citus local tables (each with bigserial / bigint DEFAULT nextval / identity columns) BEFORE promotion and assert the promoted clone's sequence windows are disjoint from the source primary's for every table kind. The reference and Citus local assertions collapse without the fix, so they guard the change. Reference-table re-ranging also newly appears in the existing add-node-from- backup tests' NOTICE output.
2 participants
citus_promote_clone_and_rebalance promotes a physical streaming replica to a new primary with a freshly-allocated group id, but the clone's sequence objects are byte-for-byte copies of the source primary's and keep the old primary's (groupId << 48) value window. The classical activation path re-ranges sequences per group id via AlterSequenceMinMax; the clone path did not, so the promoted clone and the original primary emitted values from the same window, breaking the global-uniqueness invariant for bigserial/bigint nextval(...) defaults and GENERATED ... AS IDENTITY columns on distributed tables.
Add DependentSequenceRangeAdjustCommandList() (mirrors IdentitySequenceDependencyCommandList for nextval/serial sequences) and a promotion-path helper AdjustCloneSequenceRangesForNewGroup() that re-ranges the clone's distributed-table sequences to its new group-id window. The helper runs after SyncNodeMetadataToNodes() over the reused metadata connection, so the clone's corrected local group id is visible when AlterSequenceMinMax executes.
Reuses the existing citus_internal.adjust_identity_column_seq_settings UDF; no SQL migration or catalog change. Regression coverage included