Bound ZMQ identity slot pool to cap MWorkerQueue growth under REQ churn - #69922
Open
dwoz wants to merge 1 commit into
Open
Bound ZMQ identity slot pool to cap MWorkerQueue growth under REQ churn#69922dwoz wants to merge 1 commit into
dwoz wants to merge 1 commit into
Conversation
MWorkerQueue on the master accumulates a routing-id entry in libzmq's per-peer hashtable for every unique identity a REQ client presents; the entry is retained until LINGER + TCP_KEEPALIVE timeouts expire. An unbounded process-lifetime counter meant salt-api workers, minions, or any other long-lived daemon that churns AsyncReqMessageClient instances would grow the master's routing-id table without bound. A random-per-CLI-invocation slot (previously mod-256) similarly produced one hashtable entry per CLI process on monitoring / orchestration tooling that loops on salt, salt-run, etc. Cap both slot pools with a small modulus and expose the pool size via SALT_REQ_IDENTITY_SLOT_MAX (default 8) and SALT_CLI_IDENTITY_SLOT_MAX (default 256, matching prior hardcoded value). ROUTER_HANDOVER=1 on the master swaps the older peer entry in place on slot collision; salt's existing request-timeout retry handles the (short) window where an in-flight reply is orphaned. Measured on a 4h stress rig against a 3-worker 3008.x master: MWorkerQueue RSS 541 MB -> 337 MB (-204, -38%); container mean 1010 MB -> 900 MB (-110, -11%). The characteristic step-jump pattern at ~40m and ~103m disappears; growth becomes slow-linear.
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.
Closes #69920
Summary
_REQ_IDENTITY_SLOTcounter with a mod-N pool (default 8,SALT_REQ_IDENTITY_SLOT_MAX) so a long-lived daemon that churnsAsyncReqMessageClientinstances no longer grows the master ROUTER's per-peer routing-id hashtable indefinitelyos.getpid() % 256) asSALT_CLI_IDENTITY_SLOT_MAXso operators can tune below 256 when they see MWorkerQueue growth under CLI-loop churnROUTER_HANDOVER=1(already set on the master ROUTER) swaps the older peer entry on slot collision; salt's own request-timeout retry handles the (short) window where an in-flight reply is orphanedMeasured impact
4h stress rig against a 3-worker 3008.x master (
flood_events+state.apply+state.highstate+ salt-api curl loop):The characteristic step-jump pattern at ~40m and ~103m disappears; growth becomes slow-linear.
Test plan
pytest tests/pytests/unit/transport/test_zeromq_identity_slot.py— 12 tests, all passing locallySALT_REQ_IDENTITY_SLOT_MAX=999999and confirming the counter is effectively unboundedROUTER_HANDOVER=1remains configured on the master ROUTER