Repository navigation
test: add an end-to-end benchmark to compare performance before and after a change - #1443
Merged
Merged
Conversation
…fter a change BenchmarkE2E drives an in-process standalone server through the async client with 10-byte values and up to 5,000 operations in flight, one fresh server per workload (Put, Get, Mixed80Read). It reports throughput (ns/op), p50/p99 latency and allocations, with the WAL fsync disabled. dev/bench-compare.sh builds the benchmark at a base ref and at the working tree, alternates their runs, and compares them with benchstat. Signed-off-by: Matteo Merli <mmerli@apache.org>
merlimat
requested review from
RobertIndie,
coderzc and
mattisonchao
as code owners
September 30, 2026 23:58
merlimat
added a commit
that referenced
this pull request
Oct 1, 2026
### 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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
There's no repeatable way to check whether a change makes Oxia faster or slower:
BenchmarkServer(oxiad/dataserver/benchmark_test.go) ignoresb.N: it runs the perf client for 5 minutes and then opens the pprof web UI, so its numbers can't be fed tobenchstat.oxia perfsends requests at a fixed rate and logs stats every 10 seconds, so a faster server shows the same throughput, and comparing two builds means reading logs by eye with no idea of the run-to-run noise.Example
Validating a change on the hot path today:
bin/oxiaat main, startoxia standalone, runoxia perffor a while and note the numbers in the logs.With this PR:
builds both sides, alternates their runs and prints a benchstat table. Comparing the tree against itself (6 runs per side, M1 Max) shows no false differences:
It also reports p50/p99 latency and B/op.
Modification
BenchmarkE2E(oxiad/dataserver/e2e_benchmark_test.go) starts an in-process standalone server (1 shard) with 10k keys of 10-byte values, and drives it through the async client from one goroutine with up to 5,000 operations in flight. The workloads arePut,GetandMixed80Read, each on a fresh server, so the flushes and compactions left by one don't slow down the next. It reports ns/op (the inverse of the throughput), p50/p99 latency, B/op and allocs/op; the server runs in the same process, so its allocations are included.dev/bench-compare.sh [base-ref](defaultmain) extracts the base ref withgit archive, copies the benchmark file into it (so the base doesn't need to contain it), builds both test binaries, alternates base and head runs, and compares them with benchstat.COUNT(default 6),BENCHTIME(default 3s) andBENCHtune it; a default run takes about 3 minutes.Notes
COUNT=10on a quiet machine for smaller ones.allocs/opis exact, whileB/opmoved by 1–1.5% ("significant") comparing identical code, so small B/op deltas aren't meaningful.NewSyncClientforcesWithBatchLinger(0), so each call is its own RPC and the batcher sends them one at a time. With the sync client (1 KB values), throughput stayed at ~8k ops/s from 10 to 160 concurrent callers.Mixed80Readnever filled a 1,000-op client batch, so every batch waited out the 5 ms linger.BenchmarkServeris left as is.