feat(engine): graduated Soft-pressure shed via memory limiter soft_action

[timr-dev]

What

PR1 of #2425 (follow-up to the process-wide memory limiter). Adds an opt-in
soft_action to the memory limiter policy so operators can choose to shed
ingress as soon as Soft pressure is reached, instead of waiting for
Hard.

memory_limiter:
  mode: enforce
  soft_action: shed   # default: observe

• observe (default) — byte-identical to today: Soft stays advisory, only
  Hard sheds.
• shed — reject ingress (with Retry-After) as soon as Soft pressure is
  classified.

All three should_shed_ingress() sites (process-wide MemoryPressureState,
SharedReceiverAdmissionState, LocalReceiverAdmissionState) now funnel
through a single shed_decision(mode, level, soft_action) predicate, removing
the prior copy-pasted shed condition.

Public-contract preservation

• should_shed_ingress() -> bool — signature unchanged; result byte-identical
  for the default soft_action == Observe.
• MemoryPressureChanged event payload — unchanged. soft_action is config,
  snapshotted into receiver state via from_process_state, never broadcast.
• SoftAction is a closed enum (observe / shed), so serde rejects an
  unbacked value (throttle, shed_low_priority) as an unknown enum
  variant at config load rather than silently ignoring it. Separately,
  MemoryLimiterPolicy carries #[serde(default)] + deny_unknown_fields, so
  existing configs deserialize unchanged and unknown config keys are rejected.

Performance

Criterion, single-threaded, result validated under black_box (real
materialized bool, no lazy wrapper). Compared against the committed baseline
harness (first commit in this PR).

Workload (shared / local)	Baseline	This PR	Target	Result
normal	~0.97 ns	0.868 / 0.843 ns	≤1.05×	✅ no regression
hard	~0.97 ns	0.988 / 0.975 ns	≤1.05×	✅ +1.9% / no change
soft	~0.97 ns	1.186 / 1.202 ns	n/a	⚠️ +0.22 ns (see note)

Soft-path note (honest accounting): the benchmarked ingress hot path is the
receiver-local admission state, where soft_action is a plain snapshotted
field, not an atomic — so this change adds no atomic load to the measured
path (it still does the single level atomic load it always did). The ~+0.22 ns
on Soft is extra decision logic on the Soft arm: reading the plain soft_action
byte and evaluating matches!(soft_action, Shed), work that did not exist in the
baseline where a Soft level fell through level == Hard as a single false
comparison. The normal and hard arms return without consulting soft_action,
so they show no regression. The normal "improvement" is not claimed as a
win — at sub-nanosecond scale it is within code-layout / measurement variance.

Separately, the process-wide MemoryPressureState::should_shed_ingress() API
(not the benchmarked receiver path) does add one relaxed AtomicU8 load of
soft_action; it is sub-nanosecond and off the measured hot path. A lazy load
there (only when level == Soft) was considered and deferred — the real
ingress hot path is unaffected, the load is LLVM-sinkable, and keeping a single
shed_decision predicate as the one source of truth for all three sites is
worth more than a speculative sub-ns on a non-hot path.

Memory / footprint

• No new heap allocations; nothing added to any per-request path.
• +1 AtomicU8 on the process MemoryPressureStateInner and +1 SoftAction
  (1 byte) on each receiver admission-state inner — absorbed by existing
  alignment padding. Fixed cost, O(number-of-limiters), not O(traffic).

Tests

cargo xtask check green (structure + cargo fmt + clippy --all-targets -D warnings + cargo test --workspace). +8 new tests:

Category	Tests
Contract-pinning	exhaustive 12/12 mode × level × action oracle table; the same oracle re-asserted through all three should_shed_ingress() sites (process / shared / local) to catch single-site drift; serde default + reject-unknown
Behavioral	process+receiver shed parity; ObserveOnly overrides soft_action: shed
Default-preservation	default soft_action keeps Soft advisory (byte-identical to pre-PR1)
Gap-closing	stale-generation no-op; Retry-After: 0 clamped to 1

Suite totals: engine lib 526 · config 66 (policy) · controller 105 — all pass.

Scope / follow-ups

This is the predicate + config plumbing. Deliberately not in this PR:

• Receiver coverage audit (ensuring every ingress consumer honors the new Soft
  shed) — next PR.
• Shed-event observability (metrics/logs distinguishing Soft vs Hard sheds, and
  attributing sheds to the originating receiver/signal) — follow-up PR.
• A richer graduated response (throttle / shed_low_priority) — deferred; it
  needs a rate-limit primitive and a priority model that don't exist yet, so the
  config surface intentionally rejects those values today rather than expose a
  no-op knob.

Scope: this is a process-wide backstop, not a per-source limiter

The memory limiter protects total process memory; its usage accounting and
shed decision are global and apply uniformly to all ingress — it does not
attribute usage to, or shed selectively by, individual clients/sources.
soft_action is the global default behavior. Selective per-source shedding
(rejecting only an over-limit source while others keep flowing) and per-source
attribution belong to a separate per-source resource limiter layered above
this backstop; this PR is forward-compatible with that (the flat soft_action
maps cleanly onto a future per-bucket default + overrides) and intentionally
does not pre-build it. The SoftAction doc comment records this scope inline.

Closes part of #2425.

[Copilot]

Pull request overview

Adds an opt-in soft_action to the process-wide memory limiter policy to allow ingress shedding at Soft pressure (in addition to existing Hard shedding), while preserving byte-identical default behavior (soft_action: observe). The change consolidates ingress-shed decision logic into a single predicate shared across process-wide and receiver-local admission paths, and introduces a Criterion micro-benchmark to guard hot-path performance.

Changes:

• Extend MemoryLimiterPolicy with soft_action (serde default + unknown-variant rejection via enum deserialization).
• Centralize the ingress shedding predicate via shed_decision(mode, level, soft_action) and thread soft_action through process + receiver admission states.
• Add a new memory_admission Criterion benchmark to measure should_shed_ingress() and shared-state contention behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
rust/otap-dataflow/crates/otap/src/otap_grpc.rs	Updates tests to include soft_action in MemoryPressureBehaviorConfig setup.
rust/otap-dataflow/crates/otap/src/memory_pressure_layer.rs	Updates tests to include soft_action in memory-pressure behavior configuration.
rust/otap-dataflow/crates/engine/src/memory_limiter.rs	Implements SoftAction, adds shed_decision, wires soft_action through process and receiver admission state, and adds exhaustive matrix tests.
rust/otap-dataflow/crates/engine/src/engine_metrics.rs	Updates tests to include soft_action when configuring MemoryPressureState.
rust/otap-dataflow/crates/controller/src/lib.rs	Plumbs soft_action from MemoryLimiterPolicy into the engine’s MemoryPressureBehaviorConfig.
rust/otap-dataflow/crates/config/src/policy.rs	Adds SoftAction enum and soft_action field to the memory limiter policy with defaulting behavior.
rust/otap-dataflow/benchmarks/Cargo.toml	Registers the new memory_admission benchmark target.
rust/otap-dataflow/benchmarks/benches/memory_admission/main.rs	Adds micro-benchmarks for should_shed_ingress() (shared/local) and a contention benchmark with background readers + updater.
rust/otap-dataflow/.chloggen/memory-limiter-soft-action.yaml	Adds a changelog entry for the user-facing memory limiter policy enhancement.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 99.69231% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.72%. Comparing base (c8dc3a5) to head (aa58de6).
⚠️ Report is 93 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3363      +/-   ##
==========================================
+ Coverage   86.23%   86.72%   +0.49%     
==========================================
  Files         739      789      +50     
  Lines      291248   319949   +28701     
==========================================
+ Hits       251148   277476   +26328     
- Misses      39576    41949    +2373     
  Partials      524      524              

Components	Coverage Δ
otap-dataflow	87.76% <99.69%> (+0.46%)	⬆️
query_engine	89.57% <ø> (-0.01%)	⬇️
otel-arrow-go	52.45% <ø> (ø)
quiver	92.20% <ø> (+<0.01%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[lalitb]

Nit: Can we update rust/otap-dataflow/docs/memory-limiter-phase1.md too? The “Receiver Behavior Under Hard Pressure” section still says Soft never rejects, but with soft_action: shed, Soft can now use the same receiver shedding behavior in enforce mode.

Also, should we rename memory-limiter-phase1.md to something like global-memory-limiter.md and replace the “Phase 1” wording in the title/intro with “global/ process-wide memory limiter”? This PR still changes the global limiter, while the retained-work budgeting / tenant/group isolation direction is tracked separately in #3272. A behavior-based doc name may avoid Phase 1/Phase 2 confusion as both efforts evolve.

For the same reason, could we also reword “Phase 2 of the process-wide memory limiter” in the PR description? Maybe “follow-up to the process-wide memory limiter” would be clearer.

[lalitb]

Thanks. LGTM. Good to have doc update before merge.

[timr-dev]

[timr-dev] Thanks for the review! Addressing the three points:

1. Doc accuracy — Done in 5c4873c. Renamed the section from "Receiver Behavior Under Hard Pressure" to "Receiver Behavior When Shedding Ingress" and reframed it around shedding states: ingress is shed at Hard (always, in enforce mode) and at Soft when soft_action: shed, using the same per-receiver protocol-native signals (OTLP 503 / gRPC & OTAP RESOURCE_EXHAUSTED / Syslog drops) and the same receiver-level rejection counters. The default soft_action: observe still preserves the prior Hard-only behavior, and observe_only mode still overrides soft_action and accepts all requests. Verified line-by-line against the shed_decision(mode, level, soft_action) predicate in memory_limiter.rs.

2. PR description — Done. Reworded "Phase 2 of the process-wide memory limiter" → "follow-up to the process-wide memory limiter".

3. Rename memory-limiter-phase1.md → global-memory-limiter.md + drop the "Phase 1" wording — I'd prefer to keep this out of this PR and do it as a dedicated follow-up. The rename touches cross-references in docs/configuration.md (which this PR otherwise doesn't change), and the "Phase 1 → global / process-wide" reframing is really part of the naming evolution tracked in Design and implement logical retained-work memory budgeting #3272, so it reads cleaner as its own focused change than mixing a doc-wide rename into the soft_action feature PR. I'm happy to open that follow-up right after this merges — target name global-memory-limiter.md, intro reworded to "global / process-wide memory limiter". Let me know if you'd rather I bundle it here instead.

[lalitb]

Rename memory-limiter-phase1.md → global-memory-limiter.md + drop the "Phase 1" wording — I'd prefer to keep this out of this PR and do it as a dedicated follow-up. The rename touches cross-references in docs/configuration.md (which this PR otherwise doesn't change), and the "Phase 1 → global / process-wide" reframing is really part of the naming evolution tracked in #3272, so it reads cleaner as its own focused change than mixing a doc-wide rename into the soft_action feature PR. I'm happy to open that follow-up right after this merges — target name global-memory-limiter.md, intro reworded to "global / process-wide memory limiter". Let me know if you'd rather I bundle it here instead.

Agree on this.

[timr-dev]

[timr-dev] Thanks — agreed. I'll open the dedicated follow-up right after this merges: rename memory-limiter-phase1.md → global-memory-limiter.md, rework the intro to "global / process-wide memory limiter", and update the cross-references in docs/configuration.md, tracked under #3272. Keeping this PR scoped to the soft_action feature.

[jmacd]

Reviewed critically and validated locally (checked out the branch, ran targeted tests + compiled the new bench). Approving — everything below is non-blocking.

Validation

• config 66 passed, engine::memory_limiter 24 passed (incl. all 8 new), memory_admission bench compiles, clippy clean on config.
• All three should_shed_ingress() sites and every ingress consumer (otlp/otap/syslog_cef/user_events receivers + the tower layer) route through the new predicate; no stray == Hard shed comparisons remain.
• soft_action propagation mirrors the existing mode pattern exactly (atomic on the process inner, plain field on receiver inners), and configure() runs before receivers snapshot.
• Default path is byte-identical (shed_decision(_, _, Observe) == old Enforce && Hard). Hysteresis is sound: with soft_action: shed, the existing Soft→Normal reopen threshold (soft_limit − hysteresis) becomes the shed-reopen point, so no oscillation.
• Doc sweep is thorough — grepped untouched receiver .rs/README files for stale "Soft is informational / Hard-only" wording; none remain.

Non-blocking notes

1. Process-wide MemoryPressureState::should_shed_ingress() has no production caller — receivers only consult their snapshot copies (Shared/LocalReceiverAdmissionState). The extra relaxed soft_action atomic load on that method (and the two paragraphs justifying it) doesn't touch any real hot path. Not wrong, just more analysis than the call graph warrants.

2. Wording nit in the description: soft_action: "throttle" is rejected because serde rejects unknown enum variants by default — deny_unknown_fields is a separate mechanism that governs unknown struct fields. Code and the reject-unknown test are correct; only the write-up conflates the two.

3. No end-to-end test that the HTTP/gRPC layer actually rejects at Soft when soft_action: shed. MemoryPressureLayer is the middleware that converts a shed decision into a real 503/RESOURCE_EXHAUSTED. Its only existing test, soft_pressure_remains_advisory, exercises just the default (Soft does not shed). The new shed decision is well covered — all_three_sites_agree_with_oracle_across_full_matrix asserts SharedReceiverAdmissionState::should_shed_ingress() returns true at Soft when shed, which is the exact call the layer makes. So the only thing not asserted is the trivial "layer forwards that true into a rejection" step. Low value — fine to fold into the receiver-coverage follow-up you already scoped.

4. The "Baseline" column in the perf table can't be reproduced after merge. Commit 1 adds the bench harness and commit 2 makes the behavior change, both in this PR — so there is no commit on main that has the bench but not the change. Anyone who checks out merged main and runs memory_admission gets only the "This PR" numbers. That's completely fine for the bench's actual purpose (catching future regressions against the committed code); the note is just that "Baseline" was a point-in-time dev measurement, not a number a reader can re-derive from main later.

[lquerel]

@timr-dev Overall, this looks good to me. I think there's a small issue in the logic around the soft/hard limits combined with hysteresis.

@lalit During the review of this PR, I noticed a few things related to a previous PR on the memory limiter side that seemed suboptimal. I'll file a separate report for those.

[lalitb]

@lalit During the review of this PR, I noticed a few things related to a previous PR on the memory limiter side that seemed suboptimal. I'll file a separate report for those.

Sure @lquerel. Thanks for reviewing that part. Will look forward to your findings.

[lalitb]

@lalit During the review of this PR, I noticed a few things related to a previous PR on the memory limiter side that seemed suboptimal. I'll file a separate report for those.

Sure @lquerel. Thanks for reviewing that part. Will look forward to your findings.

@lquerel - I dug a bit more into the memory limiter after your comment.

The docs describe the mechanics: sampling is periodic and enforcement happens only at ingress when the state reaches Hard. So the limitation is mostly implied rather than stated directly: this can reduce new memory growth, but it is not a strict cap and it cannot immediately reclaim memory already held inside the process.

The real issues I found are more specific:

• The default hysteresis is derived from the soft-to-hard gap and can become very large. With a wide gap, Soft can become sticky because memory must fall far below the soft limit to return to Normal. This mainly affects state reporting today because Soft does not reject ingress.
• source: auto only checks the current/leaf cgroup limit. In nested cgroups, the real limit may be on a parent, so auto detection can miss it.
• If memory sampling fails, the old pressure state stays in place. That can leave the limiter wrongly open or wrongly rejecting, with only a warning log.

I’ll anyway track a separate issue for these, but please let me know if you had a different issue in mind.

[opentelemetry-pr-dashboard]

Pull request dashboard status

Status last refreshed: 2026-07-25 23:02:00 UTC.

• Waiting on: Author
• Next step: Address or respond to 1 review feedback item:
  • Top-level feedback: 1
  • For each item, reply to move the discussion forward, e.g. link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

[timr-dev]

[timr-dev] Thanks all — addressed the outstanding feedback in aa58de6.

@lquerel (review + inline threads): the sticky-Soft hysteresis logic is fixed — the omitted-hysteresis default is now min(hard_limit − soft_limit, soft_limit / 10) instead of soft_limit − 1, so a soft_action: shed limiter recovers modestly below the soft limit instead of latching until usage nears zero. Regression + boundary tests and the soft_action-conditional doc are in the two inline threads above.

@lalitb (#issuecomment-4963445519): the first item — the default hysteresis growing large from a wide soft→hard gap and making Soft sticky — is fixed here. The other two (auto detection only reading the leaf cgroup; stale pressure state when sampling fails) are real and out of scope for this PR; agree they belong in your separate tracking issue — happy to help there.

@jmacd (review notes): (1) agreed the process-wide should_shed_ingress() has no prod caller today — left in place for state-reporting/consistency, no code change. (2) Fixed the description wording — throttle/shed_low_priority are rejected as unknown enum variants, distinct from deny_unknown_fields (which governs unknown struct keys). (3) The layer-forwards-true→503 step I'll fold into the receiver-coverage follow-up. (4) Fair — "Baseline" was a point-in-time dev measurement, not re-derivable from merged main; the bench's purpose is catching future regressions against the committed code.

[lalitb]

@timr-dev - One context update since this PR was first opened: while you were out, #3484 landed the pressure-aware rate throttling design, and #3529 is now implementing that path. That design uses Soft pressure as the trigger for selective receiver-local throttling, where over-rate scopes are throttled while within-rate scopes continue.

Given that newer direction, should we still keep soft_action: shed in this PR as a separate global Soft-pressure emergency mode? Or should this PR avoid adding that behavior now, and leave Soft-pressure throttling to the #3529 rate-throttling implementation? I realize this may mean some rework, but I think it is worth checking now so we do not land two overlapping Soft-pressure control models.

[timr-dev]

[timr-dev] @lalitb — agreed, and thanks for catching this before it landed. Two Soft-pressure control models keyed off the same signal is exactly what we want to avoid. I talked this through with my operator before replying, so this reflects a reviewed decision rather than an automated one.

Reading RFC 0002, the split is deliberate: the memory limiter stays the hard-pressure global backstop (soft = informational), and the rate policy in #3529 owns soft-pressure admission control — selectively throttling over-rate scopes while within-rate scopes continue, and reusing the same 503 / ResourceExhausted responses. A global soft_action: shed is just the blunt, all-or-nothing version of what #3529 does properly, on the identical trigger. No reason to ship both.

So I'll drop soft_action: shed from this PR and leave Soft-pressure shedding to #3529.

What I'd like to keep is the hysteresis-default bug fix (independent of soft_action): the old min(hard − soft, soft − 1) default collapsed the recovery band, so the limiter only left Soft near usage = 0. Per the RFC's Bursts and Recovery section it's the limiter's own hysteresis that governs leaving soft pressure — the very signal #3529 uses to stop throttling — so fixing it (min(hard − soft, soft / 10)) directly benefits the rate-throttling path too.

Net: this PR narrows to "fix memory-limiter hysteresis default (+docs)", ceding all Soft-pressure action to #3529. I'll rescope and re-request review. Does that match what you had in mind?

Reply posted by [GS] @timr-dev Copilot AI Assistant.