Skip to content

Fix data races and enable race detector in CI - #1750

Merged
ericyan merged 3 commits into
masterfrom
race-tests
Aug 14, 2026
Merged

Fix data races and enable race detector in CI#1750
ericyan merged 3 commits into
masterfrom
race-tests

Conversation

@ericyan

@ericyan ericyan commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

This PR enables the Go data race detector when running unit tests in the CI.

Existing data race bugs are also fixed in this PR.

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

Copilot AI balanced review requested due to automatic review settings August 13, 2026 14:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables Go race detection in CI and prevents shared GTID state from being mutated concurrently.

Changes:

  • Runs unit tests with Go’s -race flag.
  • Clones GTID state before passing it to the binlog syncer.
Show a summary per file
File Description
script/test Enables race detection for unit tests.
go/binlog/gomysql_reader.go Isolates GTID state to prevent data races.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

The binlog reader updates its current coordinates while streaming, while go-mysql's BinlogSyncer retains and mutates the GTID set passed to StartSyncGTID.

Clone the caller-provided coordinates for the reader and clone the GTID set again for the syncer. This prevents both components from mutating the caller's reconnect coordinates and eliminates concurrent access to the same MysqlGTIDSet.
The retry-hook test redirected os.Stdout and os.Stderr to capture status
and hook output. Migrate has background status and logging goroutines,
so reassigning those process-global streams races with their output.

Inject a status writer into Migrator, retaining stdout as the default.
The test supplies thread-safe buffers for the outputs, preserving the
separate stdout and stderr assertions without mutating global state.
@ericyan
ericyan requested a balanced review from Copilot August 14, 2026 13:14
@ericyan ericyan changed the title Detect data races in CI Fix data races and enable race detector in CI Aug 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@ericyan
ericyan merged commit 1cb2071 into master Aug 14, 2026
15 checks passed
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.

3 participants