mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
tlongwell-blockandDawn
4b38e7c9ea
`initialize_agent_pool` spawned and handshook each slot serially, so a pool of
N paid N x adapter-startup of dead wall-clock before it could serve anything.
With a mock adapter sleeping 3s in `initialize` and 6 workers, serial took
18275 ms; the concurrent version takes 3100 ms (5.9x of a theoretical 6x).
`AcpClient::spawn`/`initialize` hold no statics, locks, `OnceCell`, or
`env::set_var`, so there is no hidden serialization to defeat the win.
The bound is a field on `PoolStartup` (`init_concurrency`) rather than a
constant, so a future scale-from-1 pool can reuse it as its grow-batch size
without changing the signature. It is clamped to `>= 1` because a bound of zero
would deadlock every acquire.
`from_config` defaults it to 4 rather than the pool size. The measured win is
mostly retained at 4 (10 locally-installed adapter slots: ~330ms at 4, ~160ms
unbounded, ~1070ms serial), and the cap limits how many adapters handshake at
once. That matters because neither probed adapter is gateway-backed, so a
full-pool burst against a shared gateway is exactly the case the measurements
do not cover. Promoting the default toward pool size should follow that probe;
the field means doing so needs no signature change.
Three invariants a naive concurrent rewrite silently breaks:
1. Slots are positional. `AgentPool::from_slots` requires `agent.index` to
equal the agent's position in `agents`, because `return_agent` writes
`agents[agent.index]` and `crash_history[idx]` charges the circuit breaker
by the same number. A `JoinSet` yields in completion order, so results are
assigned into a pre-sized `Vec` by index and never pushed. Getting this
wrong is silent corruption, not a crash: given slots holding agents labelled
{1, 0}, a single `try_claim`/`return_agent` cycle turns two live agents into
one via `return_agent`'s "already occupied - overwriting" branch.
2. Shutdown is observed at every unbounded await, including the wait for a
permit. `acquire_init_permit` selects `shutdown.changed()` against
`Semaphore::acquire` with `biased`, so a cancelled batch's released permits
are not inherited by queued tasks that would then spawn adapter children
during teardown. Each in-flight init also reaps the child it owns rather
than being aborted from outside, and a post-loop `has_changed()` check reaps
survivors and returns `Err`, preserving the serial loop's contract that a
pool built during shutdown is never handed back as `Ok`.
3. A partial pool is valid: only an entirely dead pool is an error. Dead slots
stay present as `None`; packing them out would shift every later index.
`initialize_agent_pool` had zero test coverage - a version wrong in the first
three ways above passes 607/607 pre-existing buzz-acp tests. Ten tests now pin
it: positional placement under reversed completion order, positional partial
failure, all-dead is `Err`, wall-clock overlap, `init_concurrency` actually
bounding in-flight inits, prompt cancellation on shutdown, three for
`acquire_init_permit`'s cancellation contract, and one pinning the default
bound below pool size. Pool mocks are `bash -c` scripts, matching the existing
`spawn_script` convention.
Mutation-tested, 10 mutants all killed: push-instead-of-assign, pack-out-dead-
slots, drop the all-dead guard, revert to serial, ignore `init_concurrency`,
drop the post-loop shutdown check, remove the per-task shutdown select, ignore
shutdown while acquiring a permit, remove `biased` from that select, and revert
the default to pool size. Two needed the tests strengthened rather than the
kill claimed: removing the per-task select initially survived because the test
asserted only the error while promptness went unenforced, and the permit-wait
mutant initially hung the suite instead of failing, so that wait is now
explicitly bounded.
The permit-acquisition gap in invariant 2 was found in review by Max, who also
made the case for the conservative default.
Co-authored-by: Dawn (sprout agent) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>