fix: only enforce the per-client DNS share while the budget is scarce - #31
Merged
Merged
Conversation
#21 made the per-client bound a share of the global budget, which helped — refusals fell from 1.49/s to 0.22/s — but did not remove the pathology. 19 hours of soak: 15,175 refusals, every one of them ClientLimit, every one of them the same client, and the global budget never once reached. The share divides by the clients in the tracking table, and a client stays there for 60s after its last query, so the divisor counts clients that are present rather than clients that are contending. This bridge has five to thirteen present clients and one doing 99.6% of the queries, so the busy one was handed an eighth of a budget the others did not want, and bursts of hundreds of lookups crossed it while 512 permits sat free. Make admission work-conserving. While free global permits are above a reserve of a quarter of the budget, a client may borrow the idle capacity above its share; once the free pool falls to that reserve, every client is held to its share again. Refusing a query the gateway has the capacity to serve buys nothing, and there is nobody to starve when nobody else wants the capacity. The reserve is what keeps the anti-starvation property from #12: a client that has asked for nothing yet can still be admitted, which is asserted directly rather than implied. An operator-set `max_concurrent_queries_per_client` stays absolute and is never borrowed past. Refs #30
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.
Closes #30.
What was wrong
#21 replaced the fixed per-client constant with a share of the global budget.
Refusals fell 6.7× (1.49/s → 0.22/s) but the pathology survived. 19 hours of
soak on the same gateway:
Every refusal is
ClientLimit, every one from172.30.0.11, and the globalbudget was never once reached.
The share divides by the clients in the tracking table, where a client lingers
for 60s after its last query — so the divisor counts clients that are
present, not clients that are contending. This bridge has 5–13 present
clients and one doing 99.6% of the queries, so the busy client was handed an
eighth of a budget the others did not want. Average in-flight across the gateway
is about one query; the workload is just bursty, and bursts of hundreds crossed
a bound of 64–102 while all 512 permits sat free.
What changed
Admission is work-conserving. While free global permits are above a reserve of a
quarter of the budget, a client may borrow idle capacity above its share; once
the free pool falls to that reserve, every client is held to its share again.
The reserve is the load-bearing part: it is what keeps the anti-starvation
property that motivated #12, by guaranteeing capacity for a client that has not
asked for anything yet. There is a test asserting exactly that rather than
leaving it implied.
max_concurrent_queries_per_client, when an operator sets it, stays absoluteand is never borrowed past.
What this does not change
The global budget is still the only bound on total load, and it still refuses
with
GlobalLimitwhen genuinely exhausted. A single client on an otherwiseempty gateway could already take the whole budget after #21 — that is its share
— and still can.
Operational impact
On a single-busy-client gateway this should take per-client refusals to
approximately zero, and remove roughly half the degraded episodes in the
resolution check (
dns.queries_refusedwas 32 of 76 episodes over 4.6h).Rollback: set
dns.max_concurrent_queries_per_clientto pin a fixedabsolute ceiling, which does not borrow, or revert the commit.
Validation
cargo fmt --all --check,cargo clippy --all-targets -- -D warnings,cargo test --all-targets— all clean, 201 tests. Four new tests: idle capacitylent past the share, a burst stopping at the reserve with the share binding
again, the reserve keeping a quiet client admissible, and a fixed ceiling never
being borrowed past.