systemvm: fix subprocess deadlock in router health check scripts - #13737
Open
richevanscybermyte wants to merge 1 commit into
Open
systemvm: fix subprocess deadlock in router health check scripts#13737richevanscybermyte wants to merge 1 commit into
richevanscybermyte wants to merge 1 commit into
Conversation
The health check scripts run shell commands with Popen(stdout=PIPE) and call wait() before reading the pipe. When the command output exceeds the OS pipe capacity (64 KiB on Linux), the child blocks writing to the full pipe and the parent blocks in wait() forever. This is the deadlock warned about in the Python subprocess documentation. On routers with large iptables rule sets this makes iptables_check.py hang on every run of the advanced health check. A new stuck process chain accumulates each advanced-check interval (10 minutes by default) until the router exhausts memory and swap, keepalived's track script starves, and the router enters FAULT and reboots. On a 512 MB router this produced a reboot roughly every 9 hours, and redundant pairs failed together because their timers are synchronized at creation. Drain the pipe with communicate() and check returncode instead of calling wait() with an unread pipe. The same latent pattern is also fixed in memory_usage_check.py, cpu_usage_check.py and gateways_check.py. Fixes: apache#13735
Member
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13737 +/- ##
============================================
- Coverage 19.65% 19.64% -0.01%
+ Complexity 19792 19787 -5
============================================
Files 6368 6368
Lines 574881 574881
Branches 70351 70351
============================================
- Hits 112970 112958 -12
- Misses 449639 449650 +11
- Partials 12272 12273 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18699 |
Contributor
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
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
4 participants
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.
Description
This PR fixes #13735.
The router health check scripts run shell commands with
Popen(cmd, shell=True, stdout=PIPE)and then callpout.wait()before reading the pipe. When the command output exceeds the OS pipe capacity (64 KiB on Linux), the child process blocks writing to the full pipe and the parent blocks inwait()forever. This is the deadlock warned about in the Pythonsubprocessdocumentation.In
iptables_check.pythe command isiptables-save | grep <destIp>. On routers with large port-forwarding rule sets the output exceeds the pipe buffer and the check hangs on every run. Because the advanced health check fires everyrouter.health.checks.advanced.interval(10 minutes by default), a new stuck process chain (monitorServices.py advanced->sh->iptables_check.py) accumulates every interval, about 13 MB RSS each. On a 512 MB router, RAM and swap are exhausted after roughly 9 hours, keepalived's heartbeat track script can no longer fork within its timeout, the router enters FAULT, demotes, wipes its keepalived config and reboots. Redundant pairs fail near-simultaneously because both routers are created together, so the exhaustion clocks are synchronized. See the issue for full evidence, including a control group of routers with small rule sets running the same check cleanly for 152 days.The fix drains the pipe with
communicate()and checksreturncode, instead of callingwait()with an unread pipe. The same latent pattern is fixed inmemory_usage_check.py,cpu_usage_check.pyandgateways_check.py; their outputs are normally tiny so they do not hang today, but they share the same hazard.Types of changes
Bug Severity
How Has This Been Tested?
Reproduced on CloudStack 4.22.0.0/4.22.1.0 (KVM, redundant VPC, systemvm template 4.22.0) with a VPC whose
iptables-saveoutput is 77,029 bytes. Before the fix, every advanced health check run left a permanently stuckiptables_check.pychain (verified viaps, both processes indo_wait) and router memory climbed about 87 MB/h until the router self-rebooted at about 9 hours of uptime.With the fixed script logic (equivalent change validated on the affected router), the check completes in under a second against the same 77 KB rule set, no processes accumulate, and the router has run past the previous failure window with flat memory usage. Routers with small rule sets (1-9 KB) behave identically before and after, since their output never filled the pipe.