Skip to content

test(#834): rate-limit writers in TestMoveWithVarcharPK#835

Merged
morgo merged 1 commit into
block:mainfrom
morgo:fix-issue-834-varcharpk-rate-limit
May 11, 2026
Merged

test(#834): rate-limit writers in TestMoveWithVarcharPK#835
morgo merged 1 commit into
block:mainfrom
morgo:fix-issue-834-varcharpk-rate-limit

Conversation

@morgo

@morgo morgo commented May 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #834TestMoveWithVarcharPK timing out at 10 minutes.

The test runs 4 writer goroutines in an uncapped tight loop, each doing INSERT + UPDATE + (sometimes) DELETE per iteration with no delay. On slow CI runners this generates binlog faster than the move's reader can drain it. Inside Flush() (pkg/repl/client.go:1000) the loop never sees GetDeltaLen() < binlogTrivialThreshold because the source position keeps outrunning the buffered position. move.Run() never returns → writers never stop → wg.Wait() hangs → the 10-minute Go test timeout fires.

The MySQL 8.4 GA failure earlier today on main showed the same symptom on the other subtest (queue-full-time vs the force-enable-buffered-map one in #834), with the reader ~460 binlog files behind the live source — same root cause.

Fix

Add time.Sleep(10 * time.Millisecond) between writer iterations. Caps total throughput at ~400 ops/sec (~1200 statements/sec). The test exists to verify FIFO-queue replay correctness for VARCHAR PKs, not raw throughput, so pacing doesn't undermine its purpose.

Test plan

  • CI passes (both subtests on all MySQL versions, especially 8.4 GA which already flaked once today)
  • Spot-check locally that the test still meaningfully exercises concurrent writes (writeCount > 0 in the t.Logf output)

🤖 Generated with Claude Code

Uncapped tight-loop writers (4 goroutines, ~3 statements each, no delay)
generate binlog faster than the move's reader can drain it on slow CI
runners. Flush()'s BlockWait loop in pkg/repl/client.go never sees the
delta queue fall below binlogTrivialThreshold because the source
position keeps outrunning the buffered position. move.Run() never
returns, writers never stop, wg.Wait() hangs, and the 10-minute Go test
timeout fires.

The 8.4 GA failure earlier today showed the reader ~460 binlog files
behind the live source — same root cause, different subtest
(queue-full-time vs force-enable-buffered-map in block#834).

A 10ms sleep between iterations caps total throughput at ~400 ops/sec
(~1200 statements/sec). The test exists to verify FIFO-queue replay
correctness for VARCHAR PKs, not raw throughput, so pacing doesn't
undermine its purpose.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

Reduces flakiness/timeouts in TestMoveWithVarcharPK by pacing the concurrent writer goroutines so they can’t continuously outproduce the move’s binlog reader on slow CI runners.

Changes:

  • Adds an explanatory comment describing the root cause of the timeout (issue #834) and why pacing is acceptable for this test.
  • Inserts a time.Sleep(10 * time.Millisecond) in the writer loop to cap write throughput and allow Flush() to converge.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@morgo morgo merged commit dc7b044 into block:main May 11, 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.

Test timeout: TestMoveWithVarcharPK

3 participants