Context
Malachi.BrokerServer.reconcile_metadata/1 (lib/malachi/broker_server.ex:776-792) runs on the reconcile timer, which is :brokers_refresh_interval and defaults to 1000ms (:50, :288, :721). On every tick, after refreshing the metadata cache, it calls recover_range_state/1 (:732).
recover_range_state/1 walks every range of every topic and, for each range with an active segment, calls safe_end_offset/2 (:770), which is a ReplicationServer.end_offset/3 with a 250ms timeout to that range's primary.
So the cost of a tick is number of active ranges x round trip to their primaries, with a 250ms ceiling per range, executed inside the BrokerServer's own process loop. Two things follow:
- The loop blocks. Everything the BrokerServer serves waits behind the reconcile: with 100 active ranges and primaries that are merely slow (not dead), a pass can exceed its own 1s interval, and the timers then queue. With a handful of unreachable primaries the 250ms timeouts alone are enough to do it.
- The work is redundant. The offsets map is monotone (seeded with
max), and the reason it exists is stated right there in the code: a frontend's read horizon only advances for produces IT handled, so writes through other frontends stay invisible. That is a real requirement, but it does not require re-probing every range every second. In the steady state, a range this node is actively serving already has a current offset, and a range it is not serving does not need one until a read arrives for it.
The pattern to compare against is the one the scrub uses: bounded work per tick, with the cycle providing the coverage.
Plan
Options worth weighing before implementing, since this is a design change and not a fix:
A. Probe lazily, on the read path. Seed a range's offset when a read for it finds the local horizon behind the metadata, instead of on a timer. Turns O(ranges) per second into O(1) per stale read, and the cost lands on the request that needs it. The risk is adding a cross-node call to a read path that currently has none, which needs its own latency measurement.
B. Bound the probe per tick. Keep the timer, walk the ranges a batch at a time with a pending cycle, exactly like Malachi.Cluster.Scrubber. Cheapest to build and it composes with the existing shape, but coverage becomes a period, so a range at the tail of the cycle can carry a stale horizon for a while.
C. Take the probe off the loop. Do the same work in a separate task, sending the results back as a message the BrokerServer merges. Fixes the blocking without changing the coverage, but adds a concurrency seam and needs care so two in-flight probes cannot seed a rewind (the merge is monotone, which helps).
D. Do nothing, and document the bound. Defensible only if the intended deployment size keeps the range count small, which is not a decision to make implicitly.
Risks and open questions
- Any option that changes when the offsets map advances risks a read horizon that lags a produce through another frontend, which is the exact bug the current code prevents. Whatever is chosen needs a test that pins that behaviour first.
- The 250ms timeout is a bandaid over the same problem: it caps a single probe, not a pass. Raising the range count defeats it.
- Measure before choosing: the actual per-tick cost at a realistic range count is unknown, and it decides whether this is urgent or merely untidy.
benchmark/docker-cluster.sh already builds a multi-topic cluster and is the natural place to measure it.
Verification
- A test pinning the current guarantee (a produce through frontend A becomes readable through frontend B) before the change, so the refactor cannot quietly drop it.
- A measurement of reconcile pass duration against range count, before and after.
- Full suite,
mix credo --strict, mix dialyzer, mix docs --warnings-as-errors, single-node and 3-node loadtests with zero errors.
Found during the CodeRabbit review of PR #44.
Context
Malachi.BrokerServer.reconcile_metadata/1(lib/malachi/broker_server.ex:776-792) runs on the reconcile timer, which is:brokers_refresh_intervaland defaults to 1000ms (:50,:288,:721). On every tick, after refreshing the metadata cache, it callsrecover_range_state/1(:732).recover_range_state/1walks every range of every topic and, for each range with an active segment, callssafe_end_offset/2(:770), which is aReplicationServer.end_offset/3with a 250ms timeout to that range's primary.So the cost of a tick is
number of active ranges x round trip to their primaries, with a 250ms ceiling per range, executed inside the BrokerServer's own process loop. Two things follow:max), and the reason it exists is stated right there in the code: a frontend's read horizon only advances for produces IT handled, so writes through other frontends stay invisible. That is a real requirement, but it does not require re-probing every range every second. In the steady state, a range this node is actively serving already has a current offset, and a range it is not serving does not need one until a read arrives for it.The pattern to compare against is the one the scrub uses: bounded work per tick, with the cycle providing the coverage.
Plan
Options worth weighing before implementing, since this is a design change and not a fix:
A. Probe lazily, on the read path. Seed a range's offset when a read for it finds the local horizon behind the metadata, instead of on a timer. Turns O(ranges) per second into O(1) per stale read, and the cost lands on the request that needs it. The risk is adding a cross-node call to a read path that currently has none, which needs its own latency measurement.
B. Bound the probe per tick. Keep the timer, walk the ranges a batch at a time with a
pendingcycle, exactly likeMalachi.Cluster.Scrubber. Cheapest to build and it composes with the existing shape, but coverage becomes a period, so a range at the tail of the cycle can carry a stale horizon for a while.C. Take the probe off the loop. Do the same work in a separate task, sending the results back as a message the BrokerServer merges. Fixes the blocking without changing the coverage, but adds a concurrency seam and needs care so two in-flight probes cannot seed a rewind (the merge is monotone, which helps).
D. Do nothing, and document the bound. Defensible only if the intended deployment size keeps the range count small, which is not a decision to make implicitly.
Risks and open questions
benchmark/docker-cluster.shalready builds a multi-topic cluster and is the natural place to measure it.Verification
mix credo --strict,mix dialyzer,mix docs --warnings-as-errors, single-node and 3-node loadtests with zero errors.Found during the CodeRabbit review of PR #44.