Reduce coordinator metadata cache growth during shard moves#8652
Conversation
citus_move_shard_placement
DESCRIPTION: Re-range sequences to the new group id when promoting a clone node so distributed, reference and Citus local tables keep globally-unique sequence values 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 add/activate-node 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. Add SequenceRangeAdjustCommandList() (mirrors IdentitySequenceDependencyCommandList for nextval/serial sequences) and a promotion-path helper AdjustCloneSequenceRangesForNewGroup() that re-ranges the clone's sequences to its new group-id window. To stay in parity with the classical add/activate-node path, it selects tables via ShouldSyncTableMetadata(), so distributed, reference and Citus local tables are all covered. 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 so no SQL migration or catalog change.
…or (#8651) DESCRIPTION: Drop orphaned Citus local table shard copies on a worker promoted from a clone of the coordinator A clone node is a byte-for-byte physical replica, so a clone of the coordinator also carries the coordinator's Citus local table shard data. Citus local tables live only on the coordinator: the promotion shard-split (AdjustShardsForPrimaryCloneNodeSplit) relocates distributed shards and replicates reference tables, but it left the clone's physical copies of Citus local shards in place. Their metadata placement correctly stays on the coordinator, so the copies became untracked orphans on the new worker. When the split source is the coordinator, schedule cleanup of the Citus local tables' shard copies from the clone group; the shell table and its sequences remain, exactly as on any other worker. The metadata needs no adjustment, since the single placement stays on the coordinator. Add regression coverage (multi_add_node_from_backup_coordinator) that clones and promotes the coordinator with distributed, reference and Citus local tables present, asserting: the Citus local placement stays on group 0, the orphaned shard copy is cleaned up on the clone while the shell remains, the clone becomes a worker, and the coordinator keeps serving the Citus local table. Without the fix the orphaned copy lingers, so the test guards the change.
|
Thanks for the effort @copilot, but I don't think this change addresses the issue it's linked to (#8145). Path mismatch. The reporter explicitly stated the move ran with shard_transfer_mode = auto and "it used logical replication for the job." That means the exercised path is CopyShardTablesViaLogicalReplication → LogicallyReplicateShards → CompleteNonBlockingShardTransfer. This PR only modifies CopyShardTablesViaBlockWrites, which that run never enters, so against the reported repro it's effectively a no-op. The pattern is already handled on the real path. The logical-replication path already releases transient per-shard DDL memory everywhere via a localContext + per-iteration MemoryContextReset (e.g. ExecuteCreateConstraintsBackedByIndexCommands, CreatePartitioningHierarchy, index/FK creation, and the shard-table-creation loop in CopyShardTablesViaLogicalReplication itself). So the transient DDL buffering this PR removes isn't where the reported memory goes. Data doesn't flow through the coordinator either. CopyShardsToNode dispatches SELECT worker_copy_table_to_node(...) tasks; the bytes are copied worker→worker. So a 70 GB coordinator peak is not data buffering, and it's not the per-shard command lists (a few KB × shards = MBs, not tens of GBs). Given all transient contexts are already reset, the growth almost certainly lives in a session-lifetime context (CacheMemoryContext — relcache/catcache + Citus CitusTableCacheEntry for the whole colocation group of a deeply-partitioned 80 TB DB), or a genuine leak into a long-lived context, neither touched here. Suggested next step before iterating on a fix: reproduce with logical replication on a many-partition colocation group and capture, during the climb: SELECT * FROM pg_backend_memory_contexts ORDER BY total_bytes DESC LIMIT 30; (or a MemoryContextStats(TopMemoryContext) dump). That will show which context actually dominates and point the fix at the right place. The block-writes cleanup itself looks safe and reasonable as a standalone minor improvement, but I'd unlink it from #8145 rather than close the issue with it. |
Addressed by reverting the block-writes-only change in commit |
citus_move_shard_placement
Avoid populating full table cache entries when shard transfer only needs colocated intervals or placement metadata. Use resettable contexts for per-shard catalog reads to limit retained coordinator memory during logical moves. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5252196-f883-432f-970a-c1fd4f02e919
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5252196-f883-432f-970a-c1fd4f02e919
ihalatci
changed the title
|
@codeforall how about this attempt? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8652 +/- ##
=======================================
Coverage 88.72% 88.72%
=======================================
Files 288 288
Lines 64385 64469 +84
Branches 8108 8119 +11
=======================================
+ Hits 57125 57200 +75
- Misses 4915 4924 +9
Partials 2345 2345 🚀 New features to boost your workflow:
|
Description
Reduce coordinator memory retained during shard transfers, including the logical-replication path reported in #8145.
Large colocation groups caused shard-transfer helpers to populate full session-lifetime
CitusTableCacheEntryobjects when they only needed one colocated shard interval or a shard placement. The change uses direct catalog reads for those narrow lookups and resets temporary memory between tables/shards.Changes
Validation
Compared unpatched and patched builds under PostgreSQL 16.14 in WSL using fresh coordinator backends, two workers, forced logical moves, and increasingly large partitioned colocation groups.
At the largest scale, retained
MetadataCacheMemoryContextgrowth was 50.1% lower, total measured backend memory-context growth was 39.1% lower, and backend RSS growth was 45.9% lower.Every case preserved row counts, created one target placement per colocated relation, and left no source placements. Retained metadata still grows with metadata size; this change removes a substantial amplification source rather than making memory usage flat.