Skip to content

test: fix the end-to-end benchmark setup when a member answers the election late - #1454

Merged
merlimat merged 1 commit into
oxia-db:mainfrom
merlimat:test-e2e-benchmark-late-member
Oct 1, 2026
Merged

merlimat merged 1 commit into
oxia-db:mainfrom
merlimat:test-e2e-benchmark-late-member

Conversation

@merlimat

@merlimat merlimat commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

Before the timed run, BenchmarkE2E (#1443, #1445) waits for the followers to apply the loaded keys. waitForFollowersApplied decides who leads and who follows only once, right after the 10k loading puts, and it treats any server where GetLeader succeeds as the leader.

However, NewTerm creates a fenced leader controller on every member that isn't a follower yet. A member only becomes a follower when the leader's cursor reaches it. If a member answers NewTerm after the coordinator's 100 ms grace period (quorumFencingGracePeriod), it is left out of the election. It is added back only when the coordinator retries it (fencingFailedFollowers). The loading puts need only the leader and the other follower, so they can complete before that happens. Two servers then answer GetLeader, and the setup fails.

On main this happens in about 1 of 180 cluster setups (-test.benchtime 1x -test.count 60). When it happens during dev/bench-compare.sh, set -e stops the whole comparison before benchstat.

Example

  1. A fresh cluster runs its first election. s0 and s1 answer NewTerm right away. s2 creates its fenced leader controller, but its answer arrives after the 100 ms grace period.
  2. The coordinator elects s0 with s1 as its only follower, and retries s2 in the background.
  3. The 10k loading puts complete on s0 and s1.

Before:

  1. GetLeader succeeds on s0 and also on s2 (its fenced controller). Only s1 is counted as a follower, so the setup fails:
    --- FAIL: BenchmarkE2E/Put
        e2e_benchmark_test.go:66:
            Error Trace:  oxiad/dataserver/e2e_benchmark_test.go:174
            Error:        "[0x78d5c58b2c60]" should have 2 item(s), but has 1
            Test:         BenchmarkE2E/Put
    

After:

  1. The leader is the server whose leader controller reports LEADER, which is s0. s2's controller is FENCED.
  2. After the extra put, the wait keeps polling until both s1 and s2 have a follower controller that has applied s0's commit offset. s2 gets its follower controller once the coordinator adds it back and s0's cursor reaches it.

Modification

  • The leader is now picked by Status() == proto.ServingStatus_LEADER, not by GetLeader succeeding. Its commit offset is still read before the extra put, because followers learn the commit offset only from the next append.
  • The follower check now runs inside the require.Eventually: every server other than the leader must have a follower controller (GetFollower succeeds) that has applied that offset. This replaces the require.Len on a follower list that was built once, up front.

Verification

  • Forced race: a temporary patch, not part of this PR, made every third data server answer NewTerm 1 s late, after creating its fenced leader controller.
    • Before the fix, 3 of 3 setups failed with the error above.
    • After the fix, 15 of 15 passed. They took 25 s in total, against 9 s without the patch, because each setup waited for the late member.
  • Without the patch: 360 setups (-test.count 120) on b96d9c8 and 30 more on 1ea2328 had 0 failures.
  • Comparison script: dev/bench-compare.sh runs to completion and prints the benchstat tables.
  • Lint: golangci-lint v2.13.2 (the version CI pins) reports 0 issues on ./oxiad/....

…ection late

waitForFollowersApplied classified the servers once, right after the
keys were loaded, and took any server with a leader controller as the
leader. But NewTerm creates a fenced leader controller on every member
that isn't a follower yet, and a member only becomes a follower when the
leader's cursor reaches it. A member that answers NewTerm after the
coordinator's 100ms grace period is left out of the election and is
added back only when the coordinator retries it, so the loaded puts can
complete on the leader and the other follower first. Two servers then
answered GetLeader, and the setup failed with "should have 2 item(s),
but has 1" in about 1 of 180 cluster setups, which also stopped
dev/bench-compare.sh before benchstat.

Take the leader to be the server whose leader controller is LEADER, and
wait in the Eventually until every other server has a follower
controller that has applied the leader's commit offset.

Signed-off-by: Matteo Merli <mmerli@apache.org>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:53

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.

Copilot review overview

🟢 Approval recommended

The focused benchmark-only change correctly handles fenced controllers and late follower creation.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes flaky end-to-end benchmark setup when an election participant responds late.

Changes:

  • Identifies the active leader by LEADER status.
  • Polls dynamically until every other server becomes a caught-up follower.
File Description
oxiad/​dataserver/​e2e_benchmark_test.go Makes follower synchronization resilient to late election responses.

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

@merlimat
merlimat merged commit 1f25381 into oxia-db:main Oct 1, 2026
5 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.

2 participants