[rollout] feat: add optional tags parameter to release()#5537
[rollout] feat: add optional tags parameter to release()#5537cavities12 wants to merge 3 commits intoverl-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an optional tags parameter to the release() method across SGLang, vLLM, and TRT-LLM backends, enhancing granular control over resource release while maintaining backward compatibility and including appropriate validation. A security audit confirmed that these changes do not introduce any high or critical security vulnerabilities. However, a significant maintainability concern has been identified regarding the testing strategy.
193b5f9 to
e894627
Compare
Add tags parameter to release() across all rollout backends (SGLang, vLLM, TRT-LLM), matching the existing resume(tags) signature from verl-project#1911. Callers can now selectively release ["weights"], ["kv_cache"], or both. Co-Authored-By: Claude Opus 4.6 <[email protected]>
e894627 to
9889eef
Compare
Co-Authored-By: Claude Opus 4.6 <[email protected]>
| @@ -0,0 +1,287 @@ | |||
| # Copyright 2025 Bytedance Ltd. and/or its affiliates | |||
There was a problem hiding this comment.
Thanks for your contribution!
I suggest split vllm, trtllm and sglang tests as diffferent function with arguments, and seperate them to vllm/sgl/trtllm CI test using args to control.
So that when rollout engines upgrade, we can know the API changes at time.
What does this PR do?
Adds an optional
tagsparameter torelease()across all rollout backends (SGLang, vLLM, TRT-LLM), matching the existingresume(tags)API introduced in #1911.release()currently always releases both kv_cache and weights. With this change, callers can selectively release["weights"],["kv_cache"], or both["kv_cache", "weights"](the default, preserving backward compatibility).This completes the symmetry between
resume(tags)andrelease(tags)that was left unfinished in #1911, enabling use cases like resyncing only weights afterupdate_weights()without touching kv_cache.Checklist Before Starting
resume()but leftrelease()hardcoded[{modules}] {type}: {description}Test
15 unit tests covering all 3 backends:
["weights"],["kv_cache"])ValueError)["weights"]-only raisesNotImplementedError(sleep levels don't support it)API and Usage Example
Design & Code Changes
base.pyrelease(self, tags: list[str] | None = None)sglang_rollout.pyvllm_rollout.pyNotImplementedErrorfor weights-onlytrtllm_rollout.py"weights"to internal_WEIGHTS_TAGS, validates via settest_release_tags.pyAll backends validate unknown tags with
ValueError. Tags are normalized toset()to prevent duplicates.Checklist Before Submitting
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel.