Repository navigation
[bug] Fix lora reloading from disk every step when using AsyncLLM - #41482
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the load_lora_adapter method in vllm/entrypoints/openai/models/serving.py to reset the load_inplace attribute of the lora_request object to False before caching it. This change prevents the flag from leaking into subsequent generation requests. I have no feedback to provide as there were no review comments to evaluate.
|
PTAL @jeejeelee |
Bug fixes found in #1616 While testing out the tinker CI, found a port collision bug. async def _run_server(self) -> None: """Internal method to run the HTTP server.""" # Release the port reservation right before vLLM rebinds. if self._port_reservation is not None: self._port_reservation.close() # ← step A self._port_reservation = None ... sock_addr = (self._cli_args.host, self._cli_args.port) sock = create_server_socket(sock_addr) There exists a window of time where we free port so vllm can take it, but other actors can also. Fixed this by allocating a specific, non-overlapping port range for each actor. Additionally, changed the disk reloading workaround #1606 created here to more closely mirror the upstream change vllm-project/vllm#41482, adding a new custom endpoint with the load_inplace override. --------- Signed-off-by: hao-aaron <ahao@anyscale.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This PR introduces a tinker ci, closing #1553. It uses tinker_cookbook.recipes.math_rl.train. Currently the `REWARD_MIN_VALUE` is set to 0, but will be tightened once we have a couple of runs. Added both colocated and fully async tests. https://wandb.ai/sky-posttraining-uc-berkeley/gsm8k_tinker_ci?nw=nwuserahao https://wandb.ai/sky-posttraining-uc-berkeley/gsm8k_tinker_fully_async_ci?nw=nwuserahao While testing out the tinker CI, found a port collision bug. ``` async def _run_server(self) -> None: """Internal method to run the HTTP server.""" # Release the port reservation right before vLLM rebinds. if self._port_reservation is not None: self._port_reservation.close() # ← step A self._port_reservation = None ... sock_addr = (self._cli_args.host, self._cli_args.port) sock = create_server_socket(sock_addr) ``` There exists a window of time where we free port so vllm can take it, but other actors can also. Fixed this by allocating a specific, non-overlapping port range for each actor. Additionally, changed the disk reloading workaround #1606 created here to more closely mirror the upstream change vllm-project/vllm#41482, adding a new custom endpoint with the load_inplace override. --------- Signed-off-by: hao-aaron <ahao@anyscale.com> Signed-off-by: ahao-anyscale <ahao@anyscale.com>
|
This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you! |
|
I believe this is still an issue. self.lora_requests[lora_name] = msgspec.structs.replace(
lora_request,
load_inplace=False,
)as a fix for the openai path, but e.g. llm.chat(..., lora_request=LoRARequest(..., load_inplace=True))is not covered by that. await engine_client.add_lora(
lora_request,
load_inplace=True,
)Otherwise maybe cover all the paths that use lora requests and make sure they don't persist Why was it attached to load_inplace in the first place?
Didn't fact check everything. |
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: aoshen02 <aoshen@inferact.ai>
|
/ci run |
|
✅ Triggered Buildkite CI #96607 for commit |
…lm-project#41482) Signed-off-by: hao-aaron <ahao@anyscale.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: aoshen02 <aoshen@inferact.ai>
…itself vllm-project/vllm#41482 resets load_inplace before caching the adapter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Purpose
When calling load_lora_adapter endpoint with
load_inplace:true, lora reloads from disk every generation, leading to massive slowdowns. we set lora_request.load_inplace = False after first load to so we only load it once.Reproduction:
We observe that without the fix, lora reloads from disk on every request.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.