New `--storage` flag switches keeper-bench into an in-process mode that exercises
`KeeperStorage` directly, without the rest of the keeper server (no network, no
raft, no state machine).
Threading model mirrors a simplified Keeper commit path:
* N generator threads (`--concurrency`, default 4) produce requests using the
existing `Generator` (so `setup` + `generator` config is reused).
* One "preprocess" thread calls `preprocessRequest` for every write (stands in
for the path that, in real keeper, runs under `raft_server::lock_`).
* One "commit" thread calls `processRequest` for writes. After each committed
write, with probability `1/writes_per_read_batch` it drains the pending read
queue and processes it in one `processLocalRequests` batch.
Inter-thread queues are `NonblockingBoundedQueue` (copied from commit
6a91c30115) with 100µs polling — no condition
variables. The preprocess->commit queue has a small configurable bound
(`storage.preprocess_commit_queue_size`, default 32).
Periodic reports include (for the period): committed writes/s and reads/s,
preprocess / commit-write / commit-read thread utilization, znode count, RSS,
snapshot-mode flag, and per-op-num average preprocess/process ns and ns-per-
list-name.
Every `storage.snapshot_toggle_periods` periods the runner toggles
`enableSnapshotMode`/`disableSnapshotMode` + `clearGarbageAfterSnapshot`, so the
snapshot overhead shows up in the timings.
Refactoring: `Generator`/`PathGetter` previously took a `Coordination::ZooKeeper
&` to resolve `children_of` paths. They now take a `ListChildrenFn` callback;
the existing Runner passes a lambda wrapping the ZooKeeper client, the new
StorageRunner passes a lambda that calls `KeeperStorage::processLocalRequests`.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
PR #101801 switched `keeper-bench` setup/cleanup to the native
`RemoveRecursive` Keeper request. That request is only available on
ClickHouse Keeper versions that advertise the `REMOVE_RECURSIVE`
feature flag, so using `keeper-bench` against plain ZooKeeper or an
older Keeper fails with `RemoveRecursive request type cannot be used`.
Restore the pre-#101801 behavior as a fallback: detect the feature
flag on the connected server and, if it is not enabled, use the
previous manual `list`+batched-`multi(remove)` traversal. Also add a
`use_remove_recursive` top-level config flag (default `true`) so it
can be disabled explicitly even when the server does support it.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Reset elapsed/total timers after warmup/setup so setup time is not
counted toward throughput or time limit
- Rewrite `removeRecursive` to collect deletes in a single post-order
batch (flushed every 1000 ops) instead of removing children then parent
in separate rounds, reducing the number of multi requests
- Add proper error handling for list and multi responses
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Two fixes:
1. Measure request latency in the response callback instead of when
`collect_request` drains the in-flight deque. Previously the
`Stopwatch` ran from submission until collection, which at high
`pipeline_depth` included time the request sat completed in the
deque waiting to be collected. Now a shared `Stopwatch` is captured
by the callback and read when the response actually arrives.
2. Compute RPS using wall-clock elapsed time (`Stopwatch`) instead of
deriving it from `sum(individual_latencies) / concurrency`. The old
formula did not account for pipelining: with `pipeline_depth=64` it
reported ~2k RPS while actual wall-clock throughput was ~140k RPS.
`getThroughput` now takes elapsed seconds directly, and the
`concurrency` parameter is removed from `report` and `writeJSON`.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add `// Ok` comments to `catch (...)` blocks in `Runner.cpp` that handle
exceptions via `handle_request_exception` (which logs and counts errors)
but were flagged by the `catch_all` style check.
Fix seed collision in `MultiRequestGenerator::setSeedImpl`: the inner
`RequestGetter::setSeed` distributed sub-seeds as `seed + i + 1`, which
collided with siblings assigned by the outer `RequestGetter` using the
same scheme. Use a large prime offset (500009) to separate the inner
seed namespace.
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=98906&sha=latest&name_0=PR
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Restore batched child removal in `removeRecursive` using `multi` requests
in chunks of 1000, reducing network round trips from O(N) to O(N/1000).
Add `std::lock_guard` in `Stats::report` and `Stats::writeJSON` so that
collector fields are consistently protected by `Stats::mutex` regardless
of caller context.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Fix two issues reported by Cursor Bugbot in `Stats::report` and
`requestFromLogExecutor`:
1. `Stats::report` called `getThroughput` unconditionally on both
collectors after only checking if both are zero. A log with only read
requests would hit `assert(requests != 0)` inside
`write_collector.getThroughput`. Now each collector's throughput is
computed only when its request count is non-zero.
2. `requestFromLogExecutor` only waited for the last request's promise.
When an executor processes requests across multiple ZooKeeper
connections (one per session_id), callbacks on earlier connections
could still be running on I/O threads after `last_request->wait()`
returned, causing a data race with `report`/`writeJSON` which read
`Stats` fields without `Stats::mutex`. Fixed by collecting all
per-request futures and waiting for all of them before returning.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
The producer thread was setting `warmup_complete = true` before acquiring
the mutex and calling `info->clear()`. This created a window where a
worker could observe `warmup_complete == true`, acquire the mutex first,
record post-warmup stats, and then have those stats silently discarded
when the producer subsequently called `clear()`.
Move the atomic store after `clear()` inside the locked section so that
workers can only begin recording once the stats have been reset.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The `on_failure_callbacks` (which clean up `paths_pending` in
`CreateRequestGenerator`) were moved into the response callback lambda.
If `executeGenericRequest` threw (e.g. expired connection), the lambda
was destroyed without being invoked, so the failure callbacks never ran.
This permanently leaked entries in `paths_pending`, eventually exhausting
`max_name_generation_retries`.
Fix by storing callbacks in `shared_ptr` so they remain accessible in
the catch block, and invoking failure callbacks there.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add `pipeline_depth` and `queue_depth` settings to allow asynchronous
in-flight requests per consumer thread, hiding round-trip latency
- Add `warmup_seconds` to discard early stats while caches warm up
- Add `generator.seed` for reproducible workloads
- Track per-operation-type stats (Create, Remove, Get, Set, List, Multi)
in both stderr reports and JSON output
- Add `on_failure_callbacks` to clean up pending paths on failed creates
- Replace `paths_created` set with O(1) swap-and-pop vector+index structure
for random path removal
- Fix `static` → `thread_local` for RNG instances to avoid data races
- Fix `continue_on_errors` → `continue_on_error` (match actual config key)
- Validate `remove_factor` range, zero weights, and empty path getters
- Add retry limit for unique path name generation
- Simplify `removeRecursive` and deduplicate default ACL construction
- Document all new settings in README
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Extract `ErrnoException` into `Common/ErrnoException.h` to reduce the
include footprint of `Exception.h`. This removes `<base/errnoToString.h>`
and `<cerrno>` from `Exception.h`, which is included by ~95% of all
translation units.
All files referencing `ErrnoException` now include the new header
directly. Files that used `errnoToString` without `ErrnoException`
got an explicit `<base/errnoToString.h>` include.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
