Repository navigation
test(proxy/db): treat zombies as gone when checking the killed prisma grandchild - #40296
Closed
cursor[bot] wants to merge 1 commit into
Closed
cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
… grandchild When pytest-xdist runs the proxy-infra shard, each worker inherits the CPython interpreter's default behaviour and no explicit prctl. On CI runners the nearest subreaper for a killed grandchild is a process that never wait()s for it, so `os.kill(pid, 0)` reports the zombie as alive past the 5-second window and the test times out with an ambiguous `assert False`. Zombie state means the process has exited, which is exactly what the process-group kill contract guarantees; treating a Z or X /proc entry as gone makes the check reflect what the test is really verifying instead of racing pid reaping. Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
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.
TLDR
Problem this solves:
proxy-infra / Run testsis red on many open PRs after fix(proxy): kill the whole prisma process group when a boot migration command times out #39509 mergedtest_migrate_diff_stops_at_its_budget_and_takes_its_process_tree_with_itpollsos.kill(pid, 0)on a killed grandchild, but zombies read as aliveHow it solves it:
/proc/<pid>/statusread to the conftest helper and treatState: ZorState: Xas gonetest_db_push_timeout_takes_its_process_tree_with_it, so both tests are covered by one changeUser Flow
Before: an engineer opens any PR that touches proxy code and their CI dashboard shows
proxy-infra / Run testsred even though nothing in the PR touched migration codelitellm_internal_stagingproxy-infrashard from.github/workflows/test-unit.yml, on the merged staging tip that already contains fix(proxy): kill the whole prisma process group when a boot migration command times out #39509proxy-infra / Run tests fail, withFAILED tests/test_litellm/proxy/db/test_check_migration.py::test_migrate_diff_stops_at_its_budget_and_takes_its_process_tree_with_it - AssertionError: assert False, and the same failure repeats through both automatic rerunsAfter: the same PR runs the same shard against staging that includes this fix, and CI reflects the actual PR content
litellm_internal_stagingproxy-infrashard from.github/workflows/test-unit.yml, on the merged staging tip that includes this changetest_migrate_diff_stops_at_its_budget_and_takes_its_process_tree_with_itandtest_db_push_timeout_takes_its_process_tree_with_itboth pass on the first attempt, so the shard finishes greenRelevant issues
Linear ticket
Pre-Submission checklist
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
The test lives inside pytest-xdist and cannot be curled, so proof is a standalone reproduction that mirrors the exact conditions on the failing GHA
proxy-infrashard: a parent that is a subreaper (viaprctl(PR_SET_CHILD_SUBREAPER, 1)) spawns a fake prisma CLI withstart_new_session=True, the fake CLI spawns atime.sleep(600)grandchild and records its pid, and we thenkillpg(SIGKILL)the whole group. This is what pytest-xdist workers end up looking like once they mark themselves as subreapers, which is why the grandchild lingers as a zombie under the worker instead of getting reaped by PID 1.Before (staging tip 9fffda4)
conftest.pytoday (os.kill(pid, 0)polled up to 5s)AssertionError: assert Falsefromgrandchild_is_gone(within_seconds=5))After (3dd3430)
State: Z/State: X/proc read)Truein one poll interval instead of exhausting the 5s window, which is exactly the shape the two affected tests assert againstType
🐛 Bug Fix
✅ Test
Caveats
Low
/proc/<pid>/statusread is Linux-only, which is fine becauseos.killpgin the code under test is already POSIX-only and CI runs on Linuxtests/proxy_migration_tests/test_prisma_toolchain.py::test_a_timed_out_migrate_deploy_takes_its_process_tree_with_ithas a similar_process_is_gonehelper but it is scoped to thetest-postgresjob, uses the real prisma toolchain, and is not showing this flake today, so it stays untouchedFinal Attestation