feat(runtime): config-mapped poison policy, retire the test setter#201
Open
mfw78 wants to merge 1 commit into
Open
feat(runtime): config-mapped poison policy, retire the test setter#201mfw78 wants to merge 1 commit into
mfw78 wants to merge 1 commit into
Conversation
Map the poison-pill thresholds into EngineConfig under [limits.poison], resolved via ModuleLimits::poison the same saturating way as the [limits.http] and [limits.logs] knobs: max_failures and window_secs both saturate up to 1, omitted values fall back to the production defaults of 5 traps in 600 s. The supervisor boot paths read the resolved policy from config instead of hard-coding it, and the cfg(test) with_poison_policy setter is deleted; the integration tests now inject tight thresholds through their ModuleLimits.
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.
Summary
Closes #197; part of the epic #80 train, merges in stack order.
Poison policy thresholds move from hardcoded constants into
EngineConfig, and thecfg(test)back-door setter that let tests bypass the resolver is retired.Changes
ModuleLimitsgains apoison: PoisonLimitsSectionfield (max_failures,window_secs, both optional,#[serde(default)]) alongside apoison()resolver that follows the same saturating pattern aslogs(), defaulting to the existingPOISON_MAX_FAILURES/POISON_WINDOWconstants when the section is absent.window_secsuses seconds rather than milliseconds since a ten-minute quarantine window has no need for sub-second precision, unlike the http timeout knobs.Both
max_failuresandwindow_secssaturate to a minimum of one, matching the degenerate-zero handling already used for[limits.logs], since a zero threshold or zero window would quarantine on the very first trap.Both
supervisor::bootandsupervisor::boot_singlenow readpoison_policyvialimits.poison()instead ofPoisonPolicy::default().The
#[cfg(test)] with_poison_policysetter is deleted; the two integration tests that relied on it (poison_pill_quarantines_module_after_threshold,multi_chain_poisoned_module_does_not_affect_other_chains) now inject tight thresholds throughModuleLimits/EngineConfiginstead.engine.example.tomlgains a commented[limits.poison]block documenting the two knobs.Test plan
cargo fmt --all --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-features(389 passed, 0 failed), including both e2e poison tests executing against real in-tree wasms and the three newengine_config::tests::poison_limits_*cases (default, override, zero-saturation)Notes for reviewer
The resolver returns the runtime's own
PoisonPolicytype rather than a parallelengine_config-local struct, since the poison threshold already has one canonical home inruntime::poison_policyand a second copy would just be duplication; the import is same-crate and introduces no cycle.Production behaviour is unchanged: with no
[limits.poison]section present,poison()resolves to the same constantsPoisonPolicy::default()used before, so both boot paths dispatch identically to main.One pre-existing doc nit (an off-by-one "more than max_failures" wording in the operator-facing example, inherited rather than introduced here) is left as a follow-up rather than folded into this change.
AI Assistance: Claude Code (Opus 4.8 implementation and adversarial review, Sonnet 5 PR authoring) used for the full change.