Skip to content

Add opt-in memory-headroom backpressure for EventPublisher + MWorkerQueue - #70053

Draft
dwoz wants to merge 2 commits into
saltstack:3008.xfrom
dwoz:dwoz/feat/ep-mwq-memory-headroom
Draft

Add opt-in memory-headroom backpressure for EventPublisher + MWorkerQueue#70053
dwoz wants to merge 2 commits into
saltstack:3008.xfrom
dwoz:dwoz/feat/ep-mwq-memory-headroom

Conversation

@dwoz

@dwoz dwoz commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Companion to minion-side #70038: same shape, same opt naming, same reference-memory resolution precedence — applied to the master's EventPublisher and pooled MWorkerQueue subprocesses so operators running the master under a tight cgroup limit (VMSP small = 1 GiB, k8s memory.limit) can bound in-flight work before OOM.

New master options

opt value default
event_publisher_memory_headroom "5%" / "500M" / int bytes None
event_publisher_memory_max "5G" / int bytes None
mworker_queue_memory_headroom "5%" / "500M" / int bytes None
mworker_queue_memory_max "5G" / int bytes None
event_publisher_memory_check_interval float seconds 0.5

Reference-memory precedence (identical to #70038):

  1. Explicit *_memory_max opt
  2. cgroup v2 memory.max / memory.current
  3. cgroup v1 memory.limit_in_bytes / memory.usage_in_bytes
  4. psutil.virtual_memory().total / .used

When neither opt in a pair is set, no check runs and behavior is byte-for-byte identical to the previous unbounded default. Opt-in only per the LTS convention.

Enforcement

  • EventPublisherMasterPubServerChannel._publish_daemon starts an asyncio.Event gate and a PeriodicCallback that toggles the gate every event_publisher_memory_check_interval seconds based on has_memory_headroom(). publish_payload awaits the gate before dispatching a new event. Backpressure propagates upstream via TCPPuller's inline await payload_handler(...) — producer processes (MWorkers, salt CLI, salt-run) block on their outbound send().

  • MWorkerQueue (pooled)zmq_device_pooled's Python poll loop caches the check for the same interval and, when the check fails, skips recv_multipart on the ROUTER socket for that poll iteration. Messages stay in ZMQ's ROUTER queue; once RCVHWM fills, peer send() blocks per ZMQ semantics. Worker responses (DEALER → ROUTER) are always drained so in-flight work can complete.

  • MWorkerQueue (non-pooled)zmq_device is a C-level zmq.device(zmq.QUEUE, ...) proxy with no Python hook point. Setting the opts here logs a warning at start-up and has no effect. Requires worker_pools to be set.

Coherence with #70038

Helpers are extracted into a new module salt/utils/memory.py so the master paths here and the minion path in #70038 can converge on the same code:

  • parse_size(value) — "5G" / "500M" / int → bytes
  • parse_headroom(value, reference) — "5%" / size → bytes
  • _read_cgroup_file, _parse_self_cgroup, _detect_cgroup_memory
  • resolve_memory_reference(max_opt) — precedence chain
  • has_memory_headroom(opts, headroom_opt_key, max_opt_key, subject=None) — the actual check, parameterized on opt keys so both master (EP / MWQ) and minion (existing) can call it

When #70038 lands, its private methods on Minion can be refactored in a follow-up commit to import from salt.utils.memory.

Test plan

  • Unit tests for salt/utils/memory.py — parser edge cases, cgroup v1/v2 detection with synthetic tmp_path cgroupfs (mirror patterns from Add opt-in cgroup-aware minion_memory_headroom / minion_memory_max (#69884) #70038's tests)
  • Unit tests for MasterPubServerChannel.publish_payload — gate blocks when headroom fails, unblocks when it passes, no-op when opt unset
  • Unit tests for zmq_device_pooled — skips recv_multipart when headroom check fails; drains worker responses regardless
  • Scenario test — fire event burst against a container with tight synthetic cgroup limit, verify EP RSS stays bounded and producers see slowdown (not OOM)
  • CI green

…ueue

Mirrors the minion-side pattern in saltstack#70038 (minion_memory_headroom) so
operators running the master under a tight cgroup limit (VMSP small = 1
GiB, k8s memory.limit, etc.) can bound in-flight work per subprocess
before OOM.

Four new master config options:

  event_publisher_memory_headroom   (str/int, default None)
  event_publisher_memory_max        (str/int, default None)
  mworker_queue_memory_headroom     (str/int, default None)
  mworker_queue_memory_max          (str/int, default None)

Plus a shared knob controlling the sample cadence:

  event_publisher_memory_check_interval  (float, default 0.5)

Value semantics match saltstack#70038 exactly:

* headroom: percentage string ("5%") or absolute size ("500M", "5G",
  int bytes).
* max: absolute size / int bytes override for the reference "total
  memory available".
* Reference-total precedence: max opt > cgroup v2 > cgroup v1 >
  psutil.virtual_memory().total.

When neither opt in a pair is set, no check runs and behavior is
byte-for-byte identical to the previous unbounded default. Opt-in only,
per the LTS convention (saltstack#69443 auth_retries, saltstack#69597 gpg_decrypt).

Enforcement:

* MasterPubServerChannel._publish_daemon creates an asyncio.Event gate
  (initially set = permit) and a PeriodicCallback that toggles the gate
  every event_publisher_memory_check_interval seconds based on
  has_memory_headroom(). publish_payload awaits the gate before
  dispatching a new event.  Backpressure propagates upstream via the
  puller's existing inline await.

* MWorkerQueue.zmq_device_pooled caches the check result for the same
  interval and, when the check fails, skips recv_multipart on the
  ROUTER socket for that poll iteration.  Messages stay in ZMQ's ROUTER
  queue; once RCVHWM fills, peer sends block per ZMQ semantics.  Worker
  responses (DEALER -> ROUTER) are always drained so in-flight work can
  complete.

* MWorkerQueue.zmq_device (non-pooled) is a C-level
  zmq.device(zmq.QUEUE, ...) proxy with no Python hook point; setting
  the opts here logs a warning at start-up and has no effect.

Helpers extracted to a new module salt/utils/memory.py so the master
paths here and the minion path in saltstack#70038 can converge on the same
implementation.  When saltstack#70038 lands the minion's private helpers can be
refactored to import from salt.utils.memory in a follow-up.

Docs at doc/ref/configuration/master.rst.  Tests to follow in a
subsequent commit on this branch.
@dwoz dwoz added the test:full Run the full test suite label Aug 15, 2026
@dwoz dwoz added this to the Argon v3008.3 milestone Aug 15, 2026
Covers the new salt.utils.memory helpers and the two enforcement paths
introduced in the preceding commit:

* tests/pytests/unit/utils/test_memory.py (new): 51 cases spanning
  parse_size, parse_headroom, _read_cgroup_file, _parse_self_cgroup,
  _detect_cgroup_memory, resolve_memory_reference precedence chain,
  and the full has_memory_headroom matrix (both opts unset -> True;
  psutil missing -> True; over-limit -> False + WARNING log;
  bogus headroom -> 5% fallback; exceptions swallowed).  Mirrors the
  fixture shape from saltstack#70038's tests/pytests/unit/test_minion_memory_headroom.py.

* tests/pytests/unit/channel/test_server.py: 6 cases covering the
  MasterPubServerChannel._ep_memory_gate contract -- no-opt is a permit,
  set-from-start is a no-op, cleared blocks publish_payload,
  mid-await set releases it, and the PeriodicCallback closure toggles
  the gate based on has_memory_headroom's return value.

* tests/pytests/unit/transport/test_zeromq.py: 7 cases covering
  MWorkerQueue -- the non-pooled zmq_device warning fires (and only
  fires) when an opt is set; pooled zmq_device_pooled skips ROUTER
  recv_multipart when has_memory_headroom is False, always drains
  DEALER responses (worker replies must flow), admits when headroom
  is OK, caches the check per event_publisher_memory_check_interval,
  and never calls the check on the default (unset-opts) path.
@dwoz

dwoz commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Tests added in c67ccad (branch dwoz/feat/ep-mwq-memory-headroom).

64 new cases across 3 files:

  • tests/pytests/unit/utils/test_memory.py (new, 51 cases)

    • parse_size: int/str/bool/negative/empty/garbage/non-scalar rejection
    • parse_headroom: percent/absolute/edge-cases (0%, 101%, "abc%", whitespace)
    • _read_cgroup_file: missing / IsADirectoryError / stripped-content
    • _parse_self_cgroup: v2, v1-memory, hybrid, malformed, comma-separated controllers, empty-path root
    • _detect_cgroup_memory: v1 & v2 limited/unlimited, kernel sentinel, missing files
    • resolve_memory_reference: full precedence chain (config > cgroup-v2 > cgroup-v1 > psutil), unparseable-max fall-through
    • has_memory_headroom: both-opts-None permit, missing-psutil permit, over/under-limit, bogus-headroom 5% fallback, max-only path, subject default, exception-swallowing
  • tests/pytests/unit/channel/test_server.py (extended, 6 cases)

    • _ep_memory_gate: absent-attr permit, set-permit no-op, cleared-blocks await (verified via wait_for timeout), mid-await release, PeriodicCallback closure toggle driven by has_memory_headroom
  • tests/pytests/unit/transport/test_zeromq.py (extended, 7 cases)

    • zmq_device (non-pooled): WARNING fires when opt set, does NOT fire when unset
    • zmq_device_pooled: skips ROUTER recv_multipart when headroom fails, always drains DEALER responses (worker replies flow), admits when headroom OK, caches check per interval, zero-call on default (unset-opts) path

Local run: venv310/bin/pytest --core-tests -> 64 passed.

Coverage gaps to note:

  • No functional/scenario test that exercises real cgroup pressure end-to-end on the master (would need a container or systemd slice). The minion side in Add opt-in cgroup-aware minion_memory_headroom / minion_memory_max (#69884) #70038 has one at tests/pytests/scenarios/minion_memory_headroom/; a matching master scenario could be added in a follow-up if useful.
  • Poll-loop tests mock zmq.Poller and use a fake clock rather than driving real sockets. The invariants (skip-recv, always-drain, cache-interval) are asserted at the Python level; RCVHWM backpressure behavior is a ZMQ contract and not re-tested here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant