feat(engine): adaptive buffer-sizing PID controller (#2095)#3952
Merged
Conversation
oferchen
force-pushed
the
feat/adaptive-buffer-controller-2095
branch
from
b934d7a to
3c0ba93
Compare
Add a PID-style controller that adapts per-pipeline buffer window size in response to observed throughput, per the design RFC at docs/design/adaptive-buffer-controller.md. The new module exposes AdaptiveBufferController and a builder-style ControllerConfig with Ziegler-Nichols LAN-preset gains as defaults (K_p=0.6, K_i=0.2, K_d=0.05). Internally a single std::sync::Mutex guards the integrator, previous-error sample, and previous-sample timestamp; the recommended buffer size is exposed via an AtomicUsize for lock-free reads from the hot path. The dt is clamped to [1ms, 5s] to defend against divide-by-zero on the derivative term and integral windup on a stalled producer; the integrator is independently clamped by the configured anti-windup window. Tests cover proportional growth/shrink, integral steady-state-error elimination, derivative damping on a step input, anti-windup clamping under sustained error, both buffer-size clamps, reset semantics, zero-dt safety, and a synthetic-plant convergence test against a linear-saturating throughput model that confirms the default gains settle within 10% of the setpoint inside 50 samples. Integration into write-batch sizing, BufferPool capacity hints, and multiplex frame batching is intentionally out of scope; that work is tracked by the wiring follow-up under task #2096. The RFC originally proposed crates/engine/src/pipeline/buffer_controller.rs, but the engine crate has no pipeline module today; placing the file alongside throughput.rs keeps the module tree flat and groups it with the existing throughput EMA it will eventually consume. The deviation is documented in the file's top doc comment.
oferchen
force-pushed
the
feat/adaptive-buffer-controller-2095
branch
from
3c0ba93 to
1bf8c2b
Compare
oferchen
added a commit
that referenced
this pull request
May 18, 2026
Add a PID-style controller that adapts per-pipeline buffer window size in response to observed throughput, per the design RFC at docs/design/adaptive-buffer-controller.md. The new module exposes AdaptiveBufferController and a builder-style ControllerConfig with Ziegler-Nichols LAN-preset gains as defaults (K_p=0.6, K_i=0.2, K_d=0.05). Internally a single std::sync::Mutex guards the integrator, previous-error sample, and previous-sample timestamp; the recommended buffer size is exposed via an AtomicUsize for lock-free reads from the hot path. The dt is clamped to [1ms, 5s] to defend against divide-by-zero on the derivative term and integral windup on a stalled producer; the integrator is independently clamped by the configured anti-windup window. Tests cover proportional growth/shrink, integral steady-state-error elimination, derivative damping on a step input, anti-windup clamping under sustained error, both buffer-size clamps, reset semantics, zero-dt safety, and a synthetic-plant convergence test against a linear-saturating throughput model that confirms the default gains settle within 10% of the setpoint inside 50 samples. Integration into write-batch sizing, BufferPool capacity hints, and multiplex frame batching is intentionally out of scope; that work is tracked by the wiring follow-up under task #2096. The RFC originally proposed crates/engine/src/pipeline/buffer_controller.rs, but the engine crate has no pipeline module today; placing the file alongside throughput.rs keeps the module tree flat and groups it with the existing throughput EMA it will eventually consume. The deviation is documented in the file's top doc comment.
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
1 participant
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.
Summary
docs/design/adaptive-buffer-controller.md(task Refactor xtask util helpers into modular modules #2095, follows Support daemon alias invocation #2094).AdaptiveBufferControllerand builder-styleControllerConfigincrates/engine/src/local_copy/buffer_pool/buffer_controller.rswith Ziegler-Nichols LAN-preset default gains (K_p=0.6, K_i=0.2, K_d=0.05). The recommended buffer size is exposed via anAtomicUsizefor lock-free reads; PID state is guarded by a singlestd::sync::Mutexupdated on the sample interval.[min_size, max_size]clamps mirror RFC sections 3.2-3.4. Apub(crate) observe_at(throughput, now)testing hook makes the controller deterministic; production code calls theobserve(...)wrapper that usesInstant::now().Out of scope: integration into
BufferPoolcapacity, write-batch sizing, and multiplex frame batching. That wiring lands under the follow-up task #2096.The RFC cites
pipeline/buffer_controller.rs, but the engine crate has nopipelinemodule; placing the file alongsidethroughput.rskeeps the module tree flat. The deviation is documented at the top of the new file.Test plan
buffer_controller::testscovering proportional growth/shrink, integral steady-state error elimination, derivative damping, anti-windup clamping, both buffer-size clamps, reset semantics, zero-dt safety, default-gain convergence within 50 samples on a synthetic linear-saturating plant, builder defaults, bounded-output invariant across edge inputs, and reset preserving the recommended buffer sizeInstant,AtomicUsize,Mutexare cross-platform)