Skip to content

refactor(resharding): extract tables sync to separate function - #1499

Open
meskill wants to merge 5 commits into
mainfrom
meskill-2026-09-04-test-resharding---rukl
Open

refactor(resharding): extract tables sync to separate function#1499
meskill wants to merge 5 commits into
mainfrom
meskill-2026-09-04-test-resharding---rukl

Conversation

@meskill

@meskill meskill commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

I started with extracting the logic related to table loading from orchestrator/publisher and ended up with OmniOwnership refactoring and new edge cases testing.

This PR moves the sync_table logic to separate module with omni-sharded tables distribution between streams embedded. This allows for:

  • distribute omni for both copy_data and replication. That allows to drop OmniOwnership since it's basically the same approach from another perspective - let the single stream to handle updates for omni tables to avoid multiple parallel updates to the same rows
  • add tests and handle the cases when the table changes from sharded to omni and backward.
  • even with all of this etc. there are some cases when we can deadlock (the duplicates on multiple shards that we don't expect to duplicate etc.) actually. I don't want to lose the parallelization for this cases, but also don't even want to leave things to chances, that's why I added the lock_timeout enforcement during the resharding


/// Two subscribers race on the same omni-table rows, reproducing the cross-destination deadlock.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn cross_subscriber_omni_deadlock_two_databases() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test constructs very specific environment and flow, that is not exactly how the whole flow runs during the replication.

This was helpful for debugging initially, but now it's a burden. I'm checking the full flow with more integration test integration/resharding/repro_deadlock.sh

the behaviour is replaced with shared tables_sync implementation that distributes omni tables between source shards at initialization.
this test constructs very specific environment and flow, that is not exactly how the whole flow runs.
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.60465% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...c/backend/replication/logical/subscriber/stream.rs 83.33% 1 Missing ⚠️
...dog/src/backend/replication/logical/tables_sync.rs 99.06% 1 Missing ⚠️
pgdog/src/backend/schema/mod.rs 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@meskill
meskill force-pushed the meskill-2026-09-04-test-resharding---rukl branch from 7b59707 to 5d95173 Compare September 4, 2026 17:28
@levkk

levkk commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Any chance you could add #1473 into this as well or would that be out of scope?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants