Search code, repositories, users, issues, pull requests...

@meskill meskill commented Apr 20, 2026 •

edited

Loading

Copy link

Copy Markdown

Contributor

fixes #897

meskill added 2 commits

April 20, 2026 11:07


 sharding docs

8e64d79


 fix(copy_data): add retries for copy_data command

40f7d90

meskill

meskill commented

Apr 20, 2026

pgdog/src/backend/replication/logical/publisher/parallel_sync.rs

Comment on lines 127 to 152

@@ -106,22 +146,19 @@ impl ParallelSyncManager {

self.permit.available_permits() / self.replicas.len(),

);

let mut replicas_iter = self.replicas.iter();

// Loop through replicas, one at a time.

// This works around Rust iterators not having a "rewind" function.

let replica = loop {

if let Some(replica) = replicas_iter.next() {

break replica;

} else {

replicas_iter = self.replicas.iter();

}

};

// cycle() is the idiomatic "rewind": it restarts the iterator from the

// beginning once exhausted, giving round-robin distribution across replicas.

let mut replicas_iter = self.replicas.iter().cycle();

@meskill meskill Apr 20, 2026

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.

@levkk could you please check this. Did I get the idea right about the original intention?

@blacksmith-sh

This comment has been minimized.

@codecov

codecov Bot commented Apr 20, 2026 •

edited

Loading

Copy link

Copy Markdown

Codecov Report

❌ Patch coverage is 61.70213% with 72 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...end/replication/logical/publisher/parallel_sync.rs	0.00%	39 Missing ⚠️
...src/backend/replication/logical/publisher/table.rs	0.00%	12 Missing ⚠️
pgdog/src/backend/replication/logical/status.rs	0.00%	9 Missing ⚠️
pgdog/src/backend/pool/cluster.rs	57.14%	6 Missing ⚠️
.../client/query_engine/two_pc/server_transactions.rs	0.00%	3 Missing ⚠️
pgdog/src/backend/error.rs	92.30%	1 Missing ⚠️
pgdog/src/backend/replication/logical/error.rs	97.56%	1 Missing ⚠️
pgdog/src/frontend/router/parser/statement.rs	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

levkk

levkk reviewed

Apr 21, 2026

pgdog/src/backend/pool/cluster.rs Outdated

levkk

levkk reviewed

Apr 21, 2026

pgdog/src/backend/replication/logical/publisher/parallel_sync.rs Outdated

levkk

levkk reviewed

Apr 21, 2026

[Resharding] COPY_DATA: reliability of the data sync stage #894

pgdog/src/backend/replication/logical/publisher/parallel_sync.rs Outdated

tokio::time::sleep(backoff).await;

if let Err(trunc_err) = self.table.truncate_destination(&self.dest).await {

Choose a reason for hiding this comment

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

I don't think we should do this for two reasons:

COPY is atomic and transactional: if it fails, none of the rows will be saved in the table; it will be empty when we retry
TRUNCATE is a scary command to run for now. We should add a bunch more tests and conditions that prevent this from accidentally being called on the source DB. For now, let's have the user truncate manually if this retry logic fails for some reason.

There is a chance for a race condition where the table copy completes and we get an error somewhere below, e.g., while running COMMIT, but the odds of that are slim. We should definitely account for this (and truncate), but after we implement a few "this is definitely the destination" checks. I'll write up a separate issue for this.

@meskill meskill Apr 22, 2026

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.

yes, dropped the truncate, left the comment for future

This was linked to issues Apr 21, 2026

Closed

[Resharding] Retry COPY on transient errors #897

Closed

meskill added 4 commits

April 21, 2026 21:40


 fix: remove truncate of tables

49fa573


 test: add integration test for retry of copy_data

41bd3c4


 drop parallel_connections wrapper


 ci: update jsonschema

c0b8767

@meskill meskill mentioned this pull request

feat: explain the configuration for copy_data retries pgdogdev/docs#66

Merged


 ci: fix tests

d4d3d6c

@meskill meskill marked this pull request as ready for review

April 22, 2026 14:50

levkk

levkk reviewed

pgdog/src/backend/replication/logical/subscriber/copy.rs

Ok((0, 0))

}

async fn flush(&mut self) -> Result<(usize, usize), Error> {

@levkk levkk Apr 22, 2026

Copy link

Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

send_one is actually not free! The ParallelConnection gave us I think a 30-40% copy speed boost. Have you benchmarked this? Also curious what made you refactor this one.

@meskill meskill Apr 22, 2026

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.

moved to separate pr to verify and test it #920

meskill added 2 commits

April 22, 2026 18:19


 Revert "drop parallel_connections wrapper"

18b218c

This reverts commit 5560276.


 fix ParallelConnection error

ef30ad5

levkk

levkk reviewed

pgdog/src/backend/pool/shard/monitor.rs Outdated

impl ShardMonitor {

async fn spawn(&self) {

if self.shard.comms().lsn_check_interval == Duration::MAX {

@levkk levkk Apr 22, 2026

Copy link

Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

We test this code somewhere in CI right? Just double checking. This is the replica/primary promoter, so we need to be sure its tested before tweaking it. At the very least, test it locally (with role = "auto") making sure it still works. I think this change is fine, although probably a no-op since the code below won't take any action if data provided by lsn_check_interval-controlled loop isn't provided.

@meskill meskill Apr 22, 2026

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.

removed this

levkk

levkk reviewed

pgdog/src/backend/replication/logical/status.rs

/// Prevents accumulated counts from a discarded attempt inflating totals

/// and throughput calculations across retries.

pub(crate) fn reset(&self) {

if let Some(mut state) = TableCopies::get().get_mut(self) {

@levkk levkk Apr 22, 2026

Copy link

Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

Not a bad idea to track retries, but can add that as a follow-up.

@levkk

levkk commented Apr 22, 2026

Copy link

Copy Markdown

Collaborator

The mirror test is flaky. The rest looks good to me!

@blacksmith-sh

This comment has been minimized.


 fix tests

f1f82e6

levkk

levkk approved these changes

@levkk levkk left a comment

Copy link

Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

Should be good to go!

@meskill meskill merged commit 851d230 into main