feat(resync): Add support to update snapshot config (max_parallel_workers, num_tables, rows_per_partition) during resync signal by masterashu · Pull Request #4276 · PeerDB-io/peerdb

masterashu · 2026-05-06T04:42:23Z

Tested scenarios:

Initial + CDC Pipes:

Running pipe -> Resync (working)
Paused Pipe -> Resync (working)
Snapshot -> Resync (working)

Initial only Pipes:

Completed -> Resync (working)
Failed -> Resync (working)
Snapshot -> Resync (working)

CDC Only Pipes:

Running -> Resync (working) [Does snapshot]
Paused -> Resync (working) [Does snapshot]

github-actions · 2026-05-06T05:01:29Z

🔄 Flaky Test Detected

Analysis: The failure is a transient Docker container initialization error for the mysql-gtid 6.0 lts matrix variant — no tests ran at all, all test steps were skipped, while all sibling matrix jobs passed in the same run.
Confidence: 0.92

✅ Automatically retrying the workflow

View workflow run

codecov · 2026-05-11T05:43:04Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
2278	2	2276	203

View the top 2 failed test(s) by shortest run time

github.com/PeerDB-io/peerdb/flow/e2e::TestApiPg

Stack Traces | 0.01s run time

=== RUN   TestApiPg
=== PAUSE TestApiPg
=== CONT  TestApiPg
--- FAIL: TestApiPg (0.01s)
2026/05/29 09:19:45 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/29 09:19:45 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}

github.com/PeerDB-io/peerdb/flow/e2e::TestApiPg/TestPostgresTableOIDsMigration

Stack Traces | 0.14s run time

=== RUN   TestApiPg/TestPostgresTableOIDsMigration
=== PAUSE TestApiPg/TestPostgresTableOIDsMigration
=== CONT  TestApiPg/TestPostgresTableOIDsMigration
2026/05/29 09:18:46 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/05/29 09:18:46 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
    api_test.go:2716: 
        	Error Trace:	.../flow/e2e/api_test.go:2716
        	Error:      	Received unexpected error:
        	            	failed to get workflow_id for flow test_postgres_table_oids_api_weodcxsd: FATAL: terminating connection due to administrator command (SQLSTATE 57P01)
        	Test:       	TestApiPg/TestPostgresTableOIDsMigration
    api_test.go:49: begin tearing down postgres schema api_weodcxsd
--- FAIL: TestApiPg/TestPostgresTableOIDsMigration (0.14s)

View the full list of 2 ❄️ flaky test(s)

github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuitePG

Flake rate in main: 17.25% (Passed 355 times, Failed 74 times)

Stack Traces | 0.01s run time

=== RUN   TestPeerFlowE2ETestSuitePG
=== PAUSE TestPeerFlowE2ETestSuitePG
=== CONT  TestPeerFlowE2ETestSuitePG
--- FAIL: TestPeerFlowE2ETestSuitePG (0.01s)

github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuitePG/Test_NaN_Infinity_PG

Flake rate in main: 18.18% (Passed 18 times, Failed 4 times)

Stack Traces | 31.1s run time

=== RUN   TestPeerFlowE2ETestSuitePG/Test_NaN_Infinity_PG
=== PAUSE TestPeerFlowE2ETestSuitePG/Test_NaN_Infinity_PG
=== CONT  TestPeerFlowE2ETestSuitePG/Test_NaN_Infinity_PG
    postgres_test.go:315: UNEXPECTED STATUS TIMEOUT STATUS_SNAPSHOT
    postgres.go:92: begin tearing down postgres schema pg_wfrqiefx
--- FAIL: TestPeerFlowE2ETestSuitePG/Test_NaN_Infinity_PG (31.14s)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

github-actions · 2026-05-11T05:47:21Z

🔄 Flaky Test Detected

Analysis: TestPostgresSSHKeepaliveLatency panicked with "watch already in progress" — a race condition in pgx's context watcher during SSH keepalive connection close — crashing the entire connectors/postgres test binary and marking all sibling tests as failed with unknown status.
Confidence: 0.82

✅ Automatically retrying the workflow

View workflow run

claude · 2026-05-12T13:47:31Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

…kers, num_tables, rows_per_partition) during resync signal

jgao54 · 2026-05-14T00:50:09Z

 		changes = append(changes, fmt.Sprintf("tables removed: %v", removedTables))
 	}

+	if update.SnapshotMaxParallelWorkers > 0 {


thanks for adding this to activity logging as well!

nit: for same of completion, let's include batchSize, idleTimeout, snapshotNumPartitionsOverride to match LogActivityUpdateFlowConfig

// Only logging fields that don't need old values to compare to

The caller here will actually pass all values, changed and unchanged, and the flow-api can't distinguish which ones to log. Logging the list of tables has to happen here because they're not applied to the config until snapshotting is done, so keeping just tables and leaving the rest to the worker is better.

jgao54 · 2026-05-14T05:11:34Z

+				// Override Snapshot Parameters
+				overrideSnapshotParametersInResync(req, config)


nit: i may regret saying this (lol) but for now we are not validating these snapshot parameters so we could just remove it altogether.

maybe can add a comment here mentioning that. this avoids having update these config fields.

jgao54 · 2026-05-14T05:16:57Z

missing e2e test for this to ensure that resync propagates snapshot config changes

pfcoperez

Changes LGTM except for the seemingly unused method. I also wonder if we can unit test the config extraction, basically checking the right fields are extracted from a test.

jgao54

lgtm as well once @pfcoperez 's comment is addressed :) !

github-actions · 2026-05-22T08:27:47Z

❌ Test Failure

Analysis: The newly-added TestResyncWithSnapshotConfigOverride fails deterministically across all 9 matrix combinations (3 DB versions x 3 API sub-suites) with the same logical error "unable to signal workflow update: workflow execution already completed" — indicating a real bug in the new resync-signal feature on this PR, not transient flakiness.
Confidence: 0.92

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

github-actions · 2026-05-27T14:32:45Z

❌ Test Failure

Analysis: TestResyncWithSnapshotConfigOnResyningPipe fails identically across MySQL/Postgres/Mongo with "UNEXPECTED TIMEOUT wait for mirror to be in cdc" — this directly tests the feature added by the PR (resync snapshot config override), indicating a real bug in the resync→cdc transition, not flakiness.
Confidence: 0.9

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

github-actions · 2026-05-27T16:24:22Z

❌ Test Failure

Analysis: TestApiMy/TestResyncWithSnapshotConfigDuringSnapshot fails deterministically across all three MySQL/MariaDB matrix jobs with the same SQL syntax error near '(1,2000) g' (PostgreSQL generate_series syntax executed against MySQL), indicating a real code bug rather than flakiness.
Confidence: 0.95

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

claude · 2026-05-28T05:09:44Z

Code review

Found 1 issue. Checked for bugs and CLAUDE.md compliance.
Bug: resync-during-resync path writes _resync-suffixed table mappings to catalog
File: flow/workflows/cdc_flow.go, line 666
The comment on line 662 says we need to NOT sync the tableMappings to catalog to preserve original names. The old code intentionally used uploadConfigToCatalog(ctx, cfg) which uploaded cfg directly - at this point cfg.TableMappings has been restored to originalTableMappings (un-suffixed) on line 660.
The new code calls syncStateToConfigProtoInCatalog(ctx, cfg, state), which internally calls updateFlowConfigWithLatestSettings (lines 44-65). At line 51, it overwrites the clone TableMappings with state.SyncFlowOptions.TableMappings - which still has the _resync suffixes applied at lines 628-631. This means: (1) The catalog will be written with _resync-suffixed table names, contradicting the intent. (2) The resyncCfg passed into DropFlowInput.FlowConnectionConfigs will also carry suffixed names - when the flow is recreated, lines 628-631 will append another _resync, resulting in double-suffixed destinations like tablename_resync_resync.
Suggested fix: Apply the snapshot overrides directly to cfg (similar to applySnapshotConfigOverrides in handler.go) and keep using uploadConfigToCatalog(ctx, cfg) for this specific path, since it must NOT pull table mappings from state.

github-actions · 2026-05-28T05:15:58Z

🔄 Flaky Test Detected

Analysis: TestGenericCH_PG/Test_Initial_Custom_Partition failed because the catalog Postgres connection was terminated mid-test with SQLSTATE 57P01 (admin_shutdown), a transient infrastructure issue rather than a code bug.
Confidence: 0.92

✅ Automatically retrying the workflow

View workflow run

github-actions · 2026-05-28T05:34:19Z

🔄 Flaky Test Detected

Analysis: Catalog Postgres connection terminated mid-test with SQLSTATE 57P01 (admin_shutdown) — a transient infrastructure error unrelated to the test's logic, failing only 1 of 2448 tests in a single shard.
Confidence: 0.9

✅ Automatically retrying the workflow

View workflow run

github-actions · 2026-05-28T05:52:23Z

🔄 Flaky Test Detected

Analysis: TestApiMy/TestResyncWithSnapshotConfigOnRunningPipe hit an "UNEXPECTED TIMEOUT wait for flow dropped" (60s) on the mysql-gtid/6.0 matrix variant only, while the maria/8.0 and mysql-pos/7.0 jobs passed — timing/async-completion flakiness rather than a logic bug.
Confidence: 0.8

✅ Automatically retrying the workflow

View workflow run

ilidemi · 2026-05-28T22:24:50Z

 		changes = append(changes, fmt.Sprintf("tables removed: %v", removedTables))
 	}

+	if update.SnapshotMaxParallelWorkers > 0 {


// Only logging fields that don't need old values to compare to

The caller here will actually pass all values, changed and unchanged, and the flow-api can't distinguish which ones to log. Logging the list of tables has to happen here because they're not applied to the config until snapshotting is done, so keeping just tables and leaving the rest to the worker is better.

ilidemi · 2026-05-28T22:41:42Z

+
+				// We still override the snapshot parameters (when resync with updated values)
+				overrideSnapshotParametersInState(val, state)
+				resyncCfg := syncStateToConfigProtoInCatalog(ctx, cfg, state)


Comments just above mention needing to not sync the tableMappings to catalog because they have _resync at the end, and syncStateToConfigProtoInCatalog does exactly that

To avoid without code duplication, can move applySnapshotConfigOverrides to internal, change it to ConfigsCore, and inline the h.ValidateCDCMirror call to be able to pass ConfigsCore earlier. Then this one can be

internal.ApplySnapshotConfigOverrides(cfg, val.GetFlowConfigUpdate().GetCdcFlowConfigUpdate()) uploadConfigToCatalog(ctx, cfg)

The whole signal + mutable state thing is a mess so I understand that it's very confusing to deal with

github-actions · 2026-05-29T09:29:40Z

🔄 Flaky Test Detected

Analysis: TestApiPg/TestPostgresTableOIDsMigration failed with PostgreSQL SQLSTATE 57P01 (terminating connection due to administrator command) — an infrastructure-level connection kill rather than a test logic failure, consistent with flaky CI behavior.
Confidence: 0.85

✅ Automatically retrying the workflow

View workflow run

masterashu mentioned this pull request May 6, 2026

feat(resync): Add support to update snapshot config (max_parallel_workers, num_tables, rows_per_partition) during resync signal #4273

Closed

masterashu force-pushed the dbi-546 branch from c396d6f to c68bd69 Compare May 11, 2026 05:26

masterashu marked this pull request as ready for review May 12, 2026 13:38

masterashu requested a review from a team as a code owner May 12, 2026 13:38

masterashu requested review from jgao54 and pfcoperez May 12, 2026 13:38

This comment was marked as resolved.

Sign in to view

masterashu added 2 commits May 13, 2026 19:22

feat(resync): Add support to update snapshot config (max_parallel_wor…

c614269

…kers, num_tables, rows_per_partition) during resync signal

Fix snapshot override parameter not picked up during existing resync

57c205f

masterashu force-pushed the dbi-546 branch from 3f9d2c9 to 57c205f Compare May 13, 2026 13:52

This comment was marked as resolved.

Sign in to view

jgao54 reviewed May 14, 2026

View reviewed changes

Comment thread flow/cmd/handler.go Outdated

Comment thread flow/cmd/handler.go Outdated

Comment thread flow/workflows/cdc_flow.go Outdated

Comment thread flow/workflows/cdc_flow.go Outdated

jgao54 reviewed May 14, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

resolved comments

44e8ffb

jgao54 reviewed May 14, 2026

View reviewed changes

Comment thread flow/workflows/cdc_flow.go

resolve comments

6924494

pfcoperez reviewed May 18, 2026

View reviewed changes

Comment thread flow/cmd/handler.go Outdated

jgao54 approved these changes May 19, 2026

View reviewed changes

masterashu and others added 2 commits May 20, 2026 18:35

remove unused function

114a505

add e2e test

b4253db

masterashu added 2 commits May 27, 2026 17:40

fix(test): Updated test for resync with snapshot parameters

9050b10

Handle snapshot completed pipes

8439ee7

Fix resync during snapshot not overriding params

33d8eca

add tests

1dd91bf

masterashu added 2 commits May 27, 2026 22:50

Fix unsupported multi row insert query for mysql

f586af9

fix lint error

3790e5c

ilidemi reviewed May 28, 2026

View reviewed changes

resolevd comments

0370fd8

masterashu merged commit fafb90f into main May 29, 2026
16 of 18 checks passed

masterashu deleted the dbi-546 branch May 29, 2026 10:10

		// Override Snapshot Parameters
		overrideSnapshotParametersInResync(req, config)

Uh oh!

Conversation

masterashu commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tested scenarios:

Initial + CDC Pipes:

Initial only Pipes:

CDC Only Pipes:

Uh oh!

github-actions Bot commented May 6, 2026

🔄 Flaky Test Detected

Uh oh!

codecov Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 2 Tests Failed:

Uh oh!

github-actions Bot commented May 11, 2026

🔄 Flaky Test Detected

Uh oh!

claude Bot commented May 12, 2026

Code review

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgao54 commented May 14, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

pfcoperez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jgao54 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 22, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 27, 2026

❌ Test Failure

Uh oh!

github-actions Bot commented May 27, 2026

❌ Test Failure

Uh oh!

claude Bot commented May 28, 2026

Code review

Uh oh!

github-actions Bot commented May 28, 2026

🔄 Flaky Test Detected

Uh oh!

github-actions Bot commented May 28, 2026

🔄 Flaky Test Detected

Uh oh!

github-actions Bot commented May 28, 2026

🔄 Flaky Test Detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 29, 2026

🔄 Flaky Test Detected

masterashu commented May 6, 2026 •

edited

Loading

codecov Bot commented May 11, 2026 •

edited

Loading