Repository navigation
[Bugfix][EC Connector] Don't create hash directories on cache-miss lookups in ECExampleConnector - #60826
Merged
Conversation
…okups has_cache_item() resolves the safetensors path via _generate_filename_debug(), which auto-creates the <mm_hash>/ directory. Every probed-but-missed hash therefore leaves an empty directory in shared storage, accumulating one dir per unique missed hash over the deployment lifetime and polluting cache inspection and any listing-based tooling. Keep auto-creation on the save path (where the directory is actually needed) and disable it on the lookup path. Closes vllm-project#60820 Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
vadiklyutiy
reviewed
Oct 10, 2026
vadiklyutiy
left a comment
vadiklyutiy
left a comment
Member
There was a problem hiding this comment.
Thank you for PR
- Shorter comments please
- Too many tests, don't worth new file. Pls consider to just pick up test from here https://github.com/vadiklyutiy/vllm/tree/lyutiydoctor/20261010-040715-vllm-60820
Keep the original one-line docstrings, pass create_folder positionally, and replace the new test file with a single test added to the existing test_ec_example_connector.py. Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
Contributor
Author
|
@vadiklyutiy Thanks for the review! kept the original one-line docstrings, and replaced the new test file with the single test picked up from your branch. |
gty111
approved these changes
Oct 11, 2026
gty111
left a comment
gty111
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Thanks for the fix.
|
✅ @zhouyou9505, CI is now available for this PR.
|
Collaborator
|
@zhouyou9505 Could you merge main and kick off CI? |
Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
Contributor
Author
|
@gty111 thx. I've finished it. |
Collaborator
|
/ci run --allow-stale |
|
✅ Triggered Buildkite CI #94104 for commit |
Contributor
Author
|
@gty111 It seems the run failed, but it's might not the PR's fault. |
Collaborator
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #94104. |
Collaborator
CI selector (shadow): 3 test steps (6 jobs) instead of 63 (79 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (3)
Would skip (today's rules run them) (60)
Would add (today's rules do not run them) (0)none AMD mirrors: would skip (66)
AMD mirrors: would add (0)none 2 changed files · base |
Collaborator
Could you merge main to include CI fix commit? |
Collaborator
|
/ci retry |
|
✅ Triggered Buildkite CI #94352 for commit |
Isotr0py
approved these changes
Oct 13, 2026
AndreasKaratzas
pushed a commit
to AndreasKaratzas/vllm
that referenced
this pull request
Oct 16, 2026
…okups in ECExampleConnector (vllm-project#60826) Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
michael-gpu
pushed a commit
to practical-inference/vllm-ampere-extended
that referenced
this pull request
Oct 20, 2026
…okups in ECExampleConnector (vllm-project#60826) Signed-off-by: zhouyou9505 <zhouyou9505@gmail.com>
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.
Purpose
ECExampleConnector.has_cache_item()resolves the safetensors path via_generate_filename_debug(), which auto-creates the<mm_hash>/directory.Since
has_cache_item()is called from the scheduler's encoder-inputadmission check and from
update_state_after_alloc()on every multimodalallocation, every probed-but-missed hash leaves an empty directory in
shared storage. On a long-lived deployment the EC storage root accumulates
one empty directory per unique missed hash, making the cache impossible to
inspect and polluting any listing-based tooling/backup jobs.
Fixes #60820
Approach
Keep auto-creation on the save path (where the directory is actually
needed) and disable it on the lookup path:
_found_match_for_mm_data()now passescreate_folder=Falseto_generate_filename_debug()._generate_filename_debug()gains acreate_folder: bool = Trueparameter, forwarded to
_generate_foldername_debug(). Defaults areunchanged, so
save_caches()/start_load_caches()behavior isidentical.
No behavioral change beyond the filesystem side effect.
Testing
test_has_cache_item_miss_creates_no_folderto the existingtests/v1/ec_connector/unit/test_ec_example_connector.py: a cache-misslookup leaves no empty folder in shared storage.
connector:
VERIFY-OK.Notes
start_load_cachesis untouched: it only loads hashes thathas_cache_item()has already confirmed to exist.