Skip to content

bugfix(hicache): Fix 'MooncakeStore' not defined error. #8668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 1, 2025
Merged

bugfix(hicache): Fix 'MooncakeStore' not defined error. #8668

merged 2 commits into from
Aug 1, 2025

Conversation

hzh0425
Copy link
Collaborator

@hzh0425 hzh0425 commented Aug 1, 2025 •
edited
Loading

Motivation

When the non-mooncake store version of the hicache L3 backend is enabled, an error will be reported:

Exception in thread Thread-11 (prefetch_thread_func):
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/sgl-workspace/hzh/sglang/python/sglang/srt/managers/cache_controller.py", line 630, in prefetch_thread_func
    if not isinstance(self.storage_backend, MooncakeStore):
NameError: name 'MooncakeStore' is not defined

cc @xiezhq-hermann

Modifications

Accuracy Test

Benchmark & Profiling

Checklist

gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 1, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @hzh0425, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

I've implemented a bug fix to resolve a NameError that was occurring when the hicache feature was used with certain storage backends. The core of the problem was an attempt to check the type of the storage backend against MooncakeStore without MooncakeStore being imported, leading to a runtime error. My changes introduce a more robust way to identify the backend type by storing its name as a string and performing string comparisons, thereby eliminating the NameError and ensuring the system operates correctly regardless of the specific backend in use.

Highlights

  • Bug Fix for NameError: I've resolved an issue where a NameError would occur because MooncakeStore was not found when hicache was enabled with hf3fs/file backends. This error happened due to an isinstance check referencing MooncakeStore when it wasn't necessarily imported.
  • Refactored Backend Type Checking: To prevent the NameError, I've refactored how the storage backend type is checked. Instead of using isinstance(self.storage_backend, MooncakeStore), I now store the backend type as a string in self.storage_backend_type and use a new helper method, is_mooncake_backend(), to perform a string comparison. This avoids the need to import MooncakeStore unless it's the active backend.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 1, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively resolves the NameError that occurred when using hicache with backends other than 'mooncake'. The fix, which involves checking the storage backend type via a string comparison instead of isinstance, is correct and well-implemented. I've added one suggestion to improve maintainability by replacing a magic string with a constant.

@hzh0425 hzh0425 changed the title bugfix(hicache): Fix 'MooncakeStore' not found error. bugfix(hicache): Fix 'MooncakeStore' not defined error. Aug 1, 2025
@xiezhq-hermann xiezhq-hermann self-assigned this Aug 1, 2025
@xiezhq-hermann xiezhq-hermann added the ready-to-merge The PR is ready to merge after the CI is green. label Aug 1, 2025
@xiezhq-hermann xiezhq-hermann enabled auto-merge (squash) August 1, 2025 22:33
@zhyncs zhyncs disabled auto-merge August 1, 2025 22:58
@zhyncs zhyncs merged commit d1c4d51 into sgl-project:main Aug 1, 2025
31 of 62 checks passed
lifuhuang pushed a commit that referenced this pull request Aug 3, 2025
@hzh0425 @lifuhuang
bugfix(hicache): Fix 'MooncakeStore' not defined error. (#8668)
9dc33f7
ShangmingCai pushed a commit that referenced this pull request Aug 5, 2025
@hzh0425 @ssssnow
bugfix(hicache): Fix 'MooncakeStore' not defined error. (#8668)
de533a8
ShangmingCai pushed a commit that referenced this pull request Aug 5, 2025
@hzh0425 @ssssnow
bugfix(hicache): Fix 'MooncakeStore' not defined error. (#8668)
c80ef93
narutolhy pushed a commit to narutolhy/sglang that referenced this pull request Aug 17, 2025
@hzh0425 @narutolhy
bugfix(hicache): Fix 'MooncakeStore' not defined error. (sgl-project#…
eccf21d 
…8668)
narutolhy pushed a commit to narutolhy/sglang that referenced this pull request Aug 18, 2025
@hzh0425 @narutolhy
bugfix(hicache): Fix 'MooncakeStore' not defined error. (sgl-project#…
bc1c2d8 
…8668)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiezhq-hermann xiezhq-hermann xiezhq-hermann approved these changes

@merrymercy merrymercy Awaiting requested review from merrymercy merrymercy is a code owner

@Ying1123 Ying1123 Awaiting requested review from Ying1123 Ying1123 is a code owner

@hnyls2002 hnyls2002 Awaiting requested review from hnyls2002 hnyls2002 is a code owner

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@xiezhq-hermann xiezhq-hermann

Labels
ready-to-merge The PR is ready to merge after the CI is green.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hzh0425 @xiezhq-hermann @zhyncs