Repository navigation
Conversation
When the backend stopped waiting for a stub that missed a health update, it still signaled the end of the execution, so the stub freed its response right after sending it and the next execution loaded that freed response. This caused wrong responses, shared memory corruption and server crashes. Wait for the stub to finish the execution before ending it and releasing the requests, and clear GPU tensors left by a response that was never loaded.
9 of 22 tasks
[Medium impact] The reviewed changes appear safe to merge. SummaryThe PR keeps an unfinished execution’s shared memory alive after a failed health check. It waits for the stub’s response before ending that execution and releasing its requests. The latest changes:
No new actionable issues were established. an1310 acknowledges that a live stub that never resumes blocks its instance. an1310 also identifies retained response factories as an existing limitation; cleanup still preserves sender-owned factories. Reviews (2) · Last reviewed commit: "fix: Release the response factories of f..." · Reviewed by Greptile |
Delete the response factories of requests whose execution the backend stopped waiting for once the stub has sent its response, so the model can be unloaded. Skip factories that the response sender already deleted with a final response, which were deleted twice when the model then raised an error, and initialize the flag in the response batch that reports deleted factories.
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.
What does the PR do?
When the stub misses a health update during an execution,
ProcessRequests()returns "Stub process '...' is not healthy." It still sends the stub the message that ends the execution. The stub then frees its response as soon as it sends it, and the next execution takes that freed response as its own. The result is wrong responses, a corrupted shared memory allocator, and server crashes (boost::interprocess::lock_exception,std::bad_alloc, out-of-bounds and JSON parse errors).The backend now keeps the shared memory of that execution and sends the error responses. It then waits until the stub has sent the response before it ends the execution and releases the requests. The stub also clears GPU tensors left by a response that was never loaded.
Response factory fixes:
is_response_factory_deletedin the response batch, which was left uninitialized. Because of it, a model that raised an error inexecute()could fail to unload, such aswrong_modelin the lifecycle tests.Checklist
Agreement
<commit_type>: <Title>pre-commit install, pre-commit run --all)Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
Where should the reviewer start?
src/python_be.cc:ModelInstanceState::FinishAbandonedExecution(),ModelInstanceState::ReleaseAbandonedResponseFactories()and the error path inTRITONBACKEND_ModelInstanceExecute().Test plan:
test_execute_unresponsiveandtest_execute_error_after_finalto the lifecycle tests in test: Cover a Python backend stub that stops responding during execute server#9063, and checks that their models unload when the server shuts down.test_execute_unresponsive, the model stops its stub duringexecute(), so the server stops waiting for the response.test_execute_error_after_final, the model sends its final response through the response sender and then raises an error, with or without stopping the stub first.test_execute_unresponsive, the next request gets "Failed to retrieve the response error." and the server then aborts.test_execute_error_after_finalcrashes the server.wrong_modeldoes not unload.nvcr.io/nvidia/tritonserver:26.04-py3, both with a CPU-only build on aarch64 and with a GPU build on x86_64 (RTX 5090).Caveats:
torch.Background
On Triton 26.04 (aarch64), the server crashed repeatedly while the host was paging heavily. A stub core showed a free-list link in the shared memory pool increased by one, at the spot where
AllocatedShmOwnership::ref_count_of a freed response lives. Before 27f04d1, the backend did not end an execution it had stopped waiting for, and it restarted the stub.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)