Skip to content

fix(gms): support SGLang 0.5.16 memory pool API - #12445

Merged
galletas1712 merged 2 commits into
mainfrom
fix/dyn-3690-sglang-gms-memory-pool
Jul 31, 2026
Merged

fix(gms): support SGLang 0.5.16 memory pool API#12445
galletas1712 merged 2 commits into
mainfrom
fix/dyn-3690-sglang-gms-memory-pool

Conversation

@galletas1712

@galletas1712 galletas1712 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Patch GMS memory accounting at SGLang 0.5.16's alloc_memory_pool API.
  • Add preloaded weight memory to the persistent baseline once per runner, preserving post-capture KV sizing.
  • Publish write-mode weights only after SGLang exits all managed GMS allocation regions, so writable buffer clones use the normal CUDA allocator after GMS reconnects read-only.
  • Add focused CPU coverage for baseline accounting, publication ordering, RW-to-RO transition, nested-region failures, and finalization cleanup.

Validation

  • Focused CPU tests — 14 passed:
    • components/src/dynamo/sglang/tests/test_sglang_gms_memory_saver.py
    • lib/gpu_memory_service/tests/test_sglang_patches.py
  • Pytest marker validation — 14 checked, 0 missing marker sets.
  • Ruff lint/format, Python syntax, and diff whitespace checks — passed.
  • Narrow H100 validation: run 305932698792 passed:
    • test_gms_basic_pause_resume_sglang
    • test_gms_shadow_engine_failover_sglang
  • H100 logs confirm deferred weight publication completed, alloc_memory_pool applied the preloaded-weight baseline adjustment, and pause/resume plus post-failover inference returned HTTP 200. All Early Flake Detection retries also passed.

Related Issues

  • No related public GitHub issue
  • Internal tracking: DYN-3690

@galletas1712
galletas1712 requested a review from a team as a code owner July 30, 2026 22:04
@github-actions github-actions Bot added the fix label Jul 30, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Open in Devin Review

Comment thread lib/gpu_memory_service/integrations/sglang/patches.py Outdated
Comment thread lib/gpu_memory_service/integrations/sglang/patches.py Outdated
Comment thread lib/gpu_memory_service/integrations/sglang/patches.py Outdated
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

patch_model_runner() now supports SGLang runners without init_memory_pool by patching alloc_memory_pool. New CPU-only tests cover legacy and fallback behavior, idempotency, retries, zero preload values, and unsupported APIs.

Changes

SGLang memory patching

Layer / File(s) Summary
alloc_memory_pool fallback patch
lib/gpu_memory_service/integrations/sglang/patches.py
Detects the alternate ModelRunner API, adjusts pre_model_load_memory once using preloaded weight size, delegates allocation, and updates patch flags.
SGLang patcher test coverage
lib/gpu_memory_service/tests/test_sglang_patches.py
Adds isolated fake-runner tests for legacy and fallback APIs, retries, idempotency, zero preload values, and unsupported runners.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the fix adding support for SGLang 0.5.16’s memory pool API.
Description check ✅ Passed The description clearly explains the changes, testing, validation results, and related-issue status, despite using different section headings than the template.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/gpu_memory_service/integrations/sglang/patches.py`:
- Around line 182-190: Update the baseline-adjustment logic around
_gms_memory_baseline_adjusted to access self.pre_model_load_memory directly and
remove the getattr fallback and numeric-type guard, so missing or invalid
baselines raise while preserving the existing one-time adjustment behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fc9f0a29-eb00-4028-8399-e4505230adf0

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc7c4e and c53907e.

📒 Files selected for processing (2)
  • lib/gpu_memory_service/integrations/sglang/patches.py
  • lib/gpu_memory_service/tests/test_sglang_patches.py

Comment thread lib/gpu_memory_service/integrations/sglang/patches.py Outdated
@datadog-official

datadog-official Bot commented Jul 30, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 43.53% (-7.58%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7ce711b | Docs | Datadog PR Page | Give us feedback!

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
@galletas1712
galletas1712 force-pushed the fix/dyn-3690-sglang-gms-memory-pool branch from c53907e to 0cba288 Compare July 30, 2026 22:43
Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
@galletas1712
galletas1712 requested a review from a team as a code owner July 31, 2026 00:20
@github-actions github-actions Bot added the backend::sglang Relates to the sglang backend label Jul 31, 2026
@galletas1712
galletas1712 merged commit 290f96b into main Jul 31, 2026
109 checks passed
@galletas1712
galletas1712 deleted the fix/dyn-3690-sglang-gms-memory-pool branch July 31, 2026 01:14
nv-nmailhot pushed a commit that referenced this pull request Aug 3, 2026
…12492)

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::sglang Relates to the sglang backend fix size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants