Skip to content

[Snapshot-Agent Memory-Regions 1/N]: Adding proto and feature flag - #163

Open
Edwinhr716 wants to merge 4 commits into
llm-d-incubation:mainfrom
Edwinhr716:memory-regions-proto-routing
Open

[Snapshot-Agent Memory-Regions 1/N]: Adding proto and feature flag#163
Edwinhr716 wants to merge 4 commits into
llm-d-incubation:mainfrom
Edwinhr716:memory-regions-proto-routing

Conversation

@Edwinhr716

@Edwinhr716 Edwinhr716 commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

adds MemoryRegion / MemoryRegionsBackendConfig (oneof field 5, snapshot_name slots) with regenerated Go/Python stubs, server routing to the new backend type, and feature flag.

Why is this change needed?

API-first slice that merges safely alone — a memory-regions request routes to an unregistered backend and fails cleanly until the backend lands

How was this tested?

  • Unit tests added/updated
  • Integration/e2e tests added/updated
  • Manual testing performed

Checklist

  • Commits are signed off (git commit -s) per DCO
  • Code follows project contributing guidelines
  • Tests pass locally (make test)
  • Linters pass (make lint)
  • Documentation updated (if applicable)

Related Issues

Summary by CodeRabbit

  • New Features

    • Added configuration support for selecting device-memory regions by process, address, and size.
    • Added optional snapshot naming for selected memory regions.
    • Added a dedicated memory-regions backend option.
  • Bug Fixes

    • Memory-regions configurations are now rejected with a clear precondition error when the feature is disabled.
    • Added independent feature-gate control for memory-regions snapshots.

…e it

Add MemoryRegion and MemoryRegionsBackendConfig to the BackendConfig
oneof (field 5) with regenerated Go and Python bindings: selective
checkpoint/restore of explicit device memory ranges of a running
process, with snapshot_name naming the snapshot slot so multiple
snapshots of one process can coexist (job_id fallback; deliberately not
the request group, which identifies related jobs for the orchestrator
and does not name agent-side storage).

Route memory_regions configs to the new BackendMemoryRegions type in
getSnapshotBackendType. No implementation registers this backend yet,
so a memory_regions request fails cleanly at backend lookup; every
other config routes exactly as before.

Signed-off-by: Edwinhr716 <edandres249@gmail.com>
…ackend

Register a MemoryRegionsBackend feature gate (alpha, default off) and
reject memory_regions configs in checkFeatureGates when it is disabled,
mirroring DirectMemoryBackend: the backend is driven by GPU-CR and
requires the workload to run under the GPU-CR preloader with a shared
checkpoint directory, so selecting it must be an explicit per-agent
opt-in. The check runs before routing, so a gated config never falls
through to another backend. Gate-independence and error-text cases are
pinned in the server gate tests.

Signed-off-by: Edwinhr716 <edandres249@gmail.com>
…tations

Signed-off-by: Edwinhr716 <edandres249@gmail.com>
@Edwinhr716
Edwinhr716 requested a review from aishukamal as a code owner August 19, 2026 18:38
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f2fbc2c-8da2-4766-a805-1e7ae40010a2

📥 Commits

Reviewing files that changed from the base of the PR and between 4f9674e and 7b866ad.

⛔ Files ignored due to path filters (1)
  • pkg/snapshot-agent/api/v1alpha1/snapshot_agent.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (7)
  • pkg/client/python/timeslice/snapshot_agent/snapshot_agent_pb2.py
  • pkg/snapshot-agent/api/v1alpha1/snapshot_agent.proto
  • pkg/snapshot-agent/backends/checkpoint.go
  • pkg/snapshot-agent/features/features.go
  • pkg/snapshot-agent/features/features_internal_test.go
  • pkg/snapshot-agent/server/server.go
  • pkg/snapshot-agent/server/server_feature_gates_internal_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The snapshot-agent API now supports explicit device-memory regions through BackendConfig.memory_regions. A disabled-by-default MemoryRegionsBackend gate validates this configuration, and the server routes enabled configurations to BackendMemoryRegions.

Changes

Memory regions backend

Layer / File(s) Summary
Memory regions schema and backend contract
pkg/snapshot-agent/api/v1alpha1/snapshot_agent.proto, pkg/client/python/timeslice/snapshot_agent/snapshot_agent_pb2.py, pkg/snapshot-agent/backends/checkpoint.go
Adds MemoryRegion, MemoryRegionsBackendConfig, and the BackendConfig.memory_regions option. Adds the BackendMemoryRegions backend type and updates the generated Python descriptor.
Memory regions feature gate
pkg/snapshot-agent/features/features.go, pkg/snapshot-agent/features/features_internal_test.go
Adds MemoryRegionsBackend and registers it as disabled by default. Updates gate string assertions.
Server validation and backend routing
pkg/snapshot-agent/server/server.go, pkg/snapshot-agent/server/server_feature_gates_internal_test.go
Rejects gated configurations with FailedPrecondition, routes enabled configurations to BackendMemoryRegions, and tests gate independence and error identifiers.

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

Merge Risk: ⚪ Minimal · up to 7b866

This PR adds the memory-regions API and routing behind a feature flag without any supported current-head merge-blocking risk; it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant SnapshotRequest
  participant SnapshotAgentServer
  participant FeatureGates
  participant BackendRouter
  SnapshotRequest->>SnapshotAgentServer: Submit memory_regions configuration
  SnapshotAgentServer->>FeatureGates: Check MemoryRegionsBackend
  FeatureGates-->>SnapshotAgentServer: Enabled or FailedPrecondition
  SnapshotAgentServer->>BackendRouter: Select BackendMemoryRegions
Loading

Possibly related PRs

Suggested reviewers: aishukamal

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the snapshot-agent memory-regions change and its proto and feature-flag additions.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Edwinhr716
Edwinhr716 requested a review from lynnl0927 August 19, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants