Skip to content

mysql_cdc(test): add connection drop integration tests#4241

Merged
mmatczuk merged 1 commit intomainfrom
mmt/mysql_dataloss_q
Apr 13, 2026
Merged

mysql_cdc(test): add connection drop integration tests#4241
mmatczuk merged 1 commit intomainfrom
mmt/mysql_dataloss_q

Conversation

@mmatczuk
Copy link
Copy Markdown
Contributor

@mmatczuk mmatczuk commented Apr 10, 2026

Summary

  • Add 4 integration tests validating no data loss during MySQL CDC connection drops and reconnections
  • Covers: mid-stream kill, binlog rotation + kill, snapshot-to-CDC transition + kill, and repeated rapid disconnections with fake rotate events
  • All tests pass across MySQL 8.0, 9.0, and 9.1 with zero data loss

Context

Investigating a reported scenario where binlog rotation + connection drop + reconnect from cached checkpoint could theoretically cause data loss if old binlog files are purged. These tests confirm the canal reconnection logic correctly resumes from the cached position without losing events.

Screenshot 2026-04-10 at 13 19 55

Comment thread internal/impl/mysql/integration_test.go Outdated
Comment on lines +1265 to +1267

go func() {
_ = streamOut.Run(t.Context())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test pattern violation: silently discarded stream.Run error

The documented async stream.Run pattern for integration tests is:

go func() {
    if err := stream.Run(t.Context()); err != nil && !errors.Is(err, context.Canceled) {
        t.Error(err)
    }
}()

Discarding the error with _ = silently swallows genuine stream failures (e.g., config errors, connection issues unrelated to the test scenario), which could mask real bugs in these reconnection tests.

This applies to all 4 new test functions (TestIntegrationMySQLCDCConnectionDrop, TestIntegrationMySQLCDCConnectionDropWithBinlogRotation, TestIntegrationMySQLCDCConnectionDropAfterSnapshot, TestIntegrationMySQLCDCRepeatedConnectionDrops).

@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Commits

LGTM

Review

Adds 4 well-structured MySQL CDC connection drop integration tests with shared helpers (killReplicationConnections, idCollector, buildCDCStream). Tests cover mid-stream kill, binlog rotation + kill, post-snapshot kill, and repeated kill cycles. Good use of integration.CheckSkip, assert.Eventually, mutex-protected collectors, and license.InjectTestService.

  1. Silently discarded stream.Run error (integration_test.go L1265-L1267): All 4 new tests use _ = streamOut.Run(t.Context()) which silently swallows all stream errors. The documented pattern requires checking for non-context.Canceled errors via t.Error(err) to avoid masking genuine failures.

@mmatczuk mmatczuk force-pushed the mmt/mysql_dataloss_q branch from fa133ef to cc9d0a0 Compare April 10, 2026 11:21
@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Commits
LGTM

Review
Adds 4 well-structured integration tests for MySQL CDC connection drop resilience, with proper helpers (killReplicationConnections, idCollector, buildCDCStream). Code correctly follows existing file patterns: integration.CheckSkip(t), service.NewStreamBuilder with license.InjectTestService, assert.Eventually for polling, proper mutex synchronization in idCollector, and context.Canceled handling in background goroutines.

LGTM

@mmatczuk mmatczuk force-pushed the mmt/mysql_dataloss_q branch from cc9d0a0 to 03c438a Compare April 10, 2026 11:24
@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Commits
LGTM

Review
Adds 4 well-structured MySQL CDC connection drop integration tests with shared helpers (killReplicationConnections, idCollector, buildCDCStream). Tests correctly use integration.CheckSkip(t), license.InjectTestService, errors.Is(err, context.Canceled) in background goroutines, assert.Eventually for polling, and mutex-guarded state. Set-based assertions properly tolerate at-least-once delivery semantics. Arithmetic for inserted IDs is consistent across all test cases.

LGTM

Add 4 integration tests validating no data loss during MySQL CDC
connection drops and reconnections:
- ConnectionDrop: mid-stream kill, no rotation
- ConnectionDropWithBinlogRotation: FLUSH BINARY LOGS + kill + recovery
- ConnectionDropAfterSnapshot: snapshot -> CDC -> kill -> recovery
- RepeatedConnectionDrops: 3 rapid kill cycles with fake rotate events

All tests use set-based assertions (tolerating at-least-once duplicates)
and verify zero data loss across MySQL 8.0, 9.0, and 9.1.
@mmatczuk mmatczuk force-pushed the mmt/mysql_dataloss_q branch from 03c438a to c3d482f Compare April 10, 2026 11:57
@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Commits
LGTM

Review
Adds 4 integration tests validating MySQL CDC connection drop and reconnection scenarios with supporting helpers (killReplicationConnections, idCollector, buildCDCStream). Tests properly use integration.CheckSkip(t), service.NewStreamBuilder(), license.InjectTestService(), assert.Eventually (no require inside), t.Context(), and errors.Is(err, context.Canceled) handling. Thread-safe ID collection via mutex is correctly encapsulated.

LGTM

@mmatczuk mmatczuk merged commit 300eddb into main Apr 13, 2026
7 checks passed
@mmatczuk mmatczuk deleted the mmt/mysql_dataloss_q branch April 13, 2026 13:15
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