Skip to content

test: run the end-to-end benchmark against a 3-server cluster - #1445

Merged
merlimat merged 2 commits into
oxia-db:mainfrom
merlimat:test-e2e-benchmark-cluster
Oct 1, 2026
Merged

merlimat merged 2 commits into
oxia-db:mainfrom
merlimat:test-e2e-benchmark-cluster

Conversation

@merlimat

@merlimat merlimat commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

BenchmarkE2E (#1443) runs against an in-process standalone server: one data server, with a shard that has no followers. Nothing on the replication path is measured: the leader never streams entries to followers, never waits for a quorum ack, and the followers' WAL and apply work never runs. A change that slows down replication, or that allocates more on the follower side, looks neutral in dev/bench-compare.sh. Real deployments run with replication factor 3.

Example

  1. A change adds an allocation per entry in the follower's apply path.
  2. Before: dev/bench-compare.sh shows no change in ns/op or allocs/op, because no follower runs.
  3. After: the followers run in the benchmark process, so the extra allocation shows up in allocs/op, and any added latency shows up in Put's ns/op and p99.

Results on an M1 Max (-benchtime 3s):

Workload ns/op Throughput p50 p99 allocs/op
Put ~6,900 ~145k/s ~4.7 ms 22–35 ms 66
Get ~3,450 ~290k/s ~2.5 ms ~5 ms 32
Mixed80Read ~4,200 ~240k/s ~3.4 ms 8–10 ms 39

Put is about 30% slower than on the standalone server (~210k/s), since each write now waits for a quorum. Get is about the same.

Modification

  • Each workload now starts a fresh in-process cluster: 3 data servers (random ports, WAL fsync off) and 1 coordinator. The coordinator uses the file metadata provider, with a cluster.yaml listing the 3 servers and the default namespace at 1 shard and replication factor 3.
  • The async client connects to one of the data servers. The initial key load retries until the shard has a leader.
  • Teardown runs in reverse order: client, then coordinator, then data servers.
  • Logging is turned off for the benchmark. The logs go to stdout, and the warnings a cluster always prints at startup and shutdown (for example, replication streams cut at close) were being written into the middle of the result lines, which benchstat can't parse. A failed operation still fails the benchmark through b.Error.

dev/bench-compare.sh needs no change. It still copies the benchmark file into the base tree, so the base ref must have the coordinator APIs the file uses.

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

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

🟡 Changes recommended

The timer may start while followers are still applying setup records, contaminating benchmark results.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates BenchmarkE2E to measure replication overhead using a three-server cluster.

Changes:

  • Starts three data servers and a file-backed coordinator.
  • Uses replication factor 3 and suppresses benchmark-disrupting logs.
  • Preserves reverse-order cleanup.
File Description
oxiad/​dataserver/​e2e_benchmark_test.go Replaces the standalone benchmark server with a replicated cluster.

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

Comment thread oxiad/dataserver/e2e_benchmark_test.go
…d run

Signed-off-by: Matteo Merli <mmerli@apache.org>
@merlimat
merlimat merged commit 90af1e9 into oxia-db:main Oct 1, 2026
5 checks passed
@merlimat
merlimat deleted the test-e2e-benchmark-cluster branch October 1, 2026 01:25
merlimat added a commit that referenced this pull request Oct 1, 2026
…ection late (#1454)

### 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:

4. `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:

4. The leader is the server whose leader controller reports `LEADER`,
which is s0. s2's controller is `FENCED`.
5. 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/...`.

Signed-off-by: Matteo Merli <mmerli@apache.org>
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