Skip to content

[Bugfix] Engine Crash When Enable Decode Offloading in PD Disagg without storage backend#20677

Closed
MMuzzammil1 wants to merge 1 commit intosgl-project:mainfrom
MMuzzammil1:fix-disagg-decode-offload
Closed

[Bugfix] Engine Crash When Enable Decode Offloading in PD Disagg without storage backend#20677
MMuzzammil1 wants to merge 1 commit intosgl-project:mainfrom
MMuzzammil1:fix-disagg-decode-offload

Conversation

@MMuzzammil1
Copy link
Copy Markdown
Contributor

@MMuzzammil1 MMuzzammil1 commented Mar 16, 2026 •
edited
Loading

Motivation

When running the decode node with --disaggregation-decode-enable-offload-kvcache flag, if there is no L3 storage backend, the Engine Crashes, because the ack_backup_queue, backup_queue of HiCacheController is None (https://github.com/sgl-project/sglang/blob/main/python/sglang/srt/disaggregation/decode_kvcache_offload_manager.py#L177, https://github.com/sgl-project/sglang/blob/main/python/sglang/srt/managers/cache_controller.py#L324).

Modifications

Added enable_storage flag. Trigger backup logic only when this flag is set.

Checklist

@MMuzzammil1
Add enable_storage flag to avoid engine crash when storage backend is…
cc6285c 
… None

Signed-off-by: Mohd Muzzammil <me.muzzammil@samsung.com>
@gemini-code-assist
Copy link
Copy Markdown
Contributor

gemini-code-assist Bot commented Mar 16, 2026

Summary of Changes

Hello, 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!

This pull request addresses a critical bug causing an engine crash when the decode offloading feature is used in a disaggregated setup without an active storage backend. The fix introduces a new flag to intelligently gate storage-dependent operations, ensuring that the system only attempts to interact with a storage backend if one has been explicitly configured, thereby enhancing stability and preventing runtime errors.

Highlights

  • Bugfix for Engine Crash: Resolved an engine crash that occurred when decode offloading was enabled in PD Disaggregation without a configured storage backend. Previously, the system would attempt to access ack_backup_queue and backup_queue which were None, leading to a crash.
  • Conditional Storage Backend Logic: Introduced an enable_storage flag, derived from server_args.hicache_storage_backend, to conditionally execute storage-related operations. This ensures that backup and hash computation logic is only triggered when a storage backend is actually present.
  • Refined KVCache Offload Management: Modified the decode_kvcache_offload_manager to guard operations like computing prefix hashes, checking backup queue sizes, and triggering backup processes with the new enable_storage flag, preventing AttributeError when no storage is configured.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • python/sglang/srt/disaggregation/decode_kvcache_offload_manager.py
    • Added self.enable_storage flag in the constructor to track the presence of a HiCache storage backend.
    • Wrapped the computation of prefill_hashes and last_prefill_hash in offload_kv_cache with a conditional check for self.enable_storage.
    • Modified check_offload_progress to conditionally include cc.ack_backup_queue.qsize() in qsizes and to conditionally call _check_backup_progress based on self.enable_storage.
    • Enclosed the logic for computing prior_hash, triggering backup, and updating last_hash within _check_offload_progress in an if self.enable_storage block.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

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 Mar 16, 2026
View reviewed changes
Copy link
Copy Markdown
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

The pull request effectively addresses the engine crash that occurred when decode offloading was enabled without an L3 storage backend. The introduction of the enable_storage flag and its consistent use across __init__, offload_kv_cache, check_offload_progress, and _check_offload_progress methods correctly gates storage-related operations. This ensures that backup logic and hash computations are only triggered when a storage backend is actually configured, preventing None object access errors. The changes are well-targeted and improve the robustness of the system.

@hzh0425
Copy link
Copy Markdown
Collaborator

hzh0425 commented Mar 16, 2026

hi @MMuzzammil1
If you don't configure a storage backend, decode offload will have no effect.
You might want to add it in server_args—configuring a storage backend is required to enable decode offload.
Other settings probably don't need to be changed.

@MMuzzammil1
Copy link
Copy Markdown
Contributor Author

MMuzzammil1 commented Mar 17, 2026

hi @MMuzzammil1 If you don't configure a storage backend, decode offload will have no effect. You might want to add it in server_args—configuring a storage backend is required to enable decode offload. Other settings probably don't need to be changed.

Thanks for the info @hzh0425. Could you please then review this PR that adds sanity check in the server args: #20732

Actually I thought decode offload might also have hierarchy of L2 and L3 in PD Disagg, but I was wrong.

@MMuzzammil1 MMuzzammil1 mentioned this pull request Mar 17, 2026
Merged
@MMuzzammil1 MMuzzammil1 closed this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

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

@ShangmingCai ShangmingCai Awaiting requested review from ShangmingCai ShangmingCai 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

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MMuzzammil1 @hzh0425