Skip to content

feat(snapshot): route opted-in checkpoints through PageBroker - #13098

Closed
dfeigin-nv wants to merge 2 commits into
pagebroker-daemon-testsfrom
pagebroker-checkpoint
Closed

feat(snapshot): route opted-in checkpoints through PageBroker#13098
dfeigin-nv wants to merge 2 commits into
pagebroker-daemon-testsfrom
pagebroker-checkpoint

Conversation

@dfeigin-nv

@dfeigin-nv dfeigin-nv commented Aug 12, 2026 •
edited by devin-ai-integration Bot
Loading

Copy link
Copy Markdown
Contributor

What

Route checkpoints through PageBroker only when the Pod annotation requests it and the deployment enables PageBroker.

  • Prepare a broker-owned tmpfs output directory before CRIU dumps.
  • Commit publishes that directory through the configured filesystem/POSIX plan.
  • Abort cleans broker staging when checkpoint capture or publication fails.
  • Retrying Commit after a lost local control-socket response is bounded to five seconds.

The existing storage-local staging and rename path is unchanged when PageBroker is not selected.

Scope

Checkpoint routing only. No restore routing, mount injection, Helm deployment, GPU path, S3, NIXL, or CRIU provider integration.

Validation

go -C deploy/snapshot test ./internal/pagebroker ./internal/executor ./internal/controller

git diff --check pagebroker-daemon...pagebroker-checkpoint

Open in Devin Review

@dfeigin-nv
dfeigin-nv requested a review from a team as a code owner August 12, 2026 14:25
@github-actions github-actions Bot added the feat label Aug 12, 2026
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from f791f86 to 4de3930 Compare August 12, 2026 14:28
@dfeigin-nv
dfeigin-nv requested a review from a team as a code owner August 12, 2026 14:28
@dfeigin-nv
dfeigin-nv changed the base branch from pagebroker-daemon to pagebroker-daemon-tests August 12, 2026 14:28
devin-ai-integration[bot]
devin-ai-integration Bot reviewed Aug 12, 2026
View reviewed changes

@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 deploy/snapshot/internal/executor/checkpoint.go Outdated
Comment thread deploy/snapshot/internal/controller/podsnapshotcontent.go Outdated
Comment thread deploy/snapshot/internal/executor/checkpoint.go
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from 4de3930 to d1d78a8 Compare August 12, 2026 14:32
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from d1d78a8 to 5f3a535 Compare August 12, 2026 14:38
dynamo-review-agent[bot]
dynamo-review-agent Bot reviewed Aug 12, 2026
View reviewed changes
Comment thread deploy/snapshot/internal/pagebroker/client.go
dynamo-review-agent[bot]
dynamo-review-agent Bot reviewed Aug 12, 2026
View reviewed changes
Comment thread deploy/snapshot/internal/executor/checkpoint.go
@datadog-official

datadog-official Bot commented Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Pipelines

⚠️ Warnings

🚦 4 Pipeline jobs failed

Pre Merge | pre-commit   View in Datadog   GitHub Actions

🔧 Fix in code. This looks caused by changes in this PR. Pre-commit hook made changes in file formatting and syntax, which were not committed. Job exited with code 1.

Pre Merge | snapshot   View in Datadog   GitHub Actions

🔧 Fix in code. This looks caused by changes in this PR. Linker error: undefined references to several functions in pagebroker causing the build to fail during testing.

Docs link check | lychee   View in Datadog   GitHub Actions

See error Multiple 404 Not Found errors encountered while accessing documentation links.

View all 4 failed jobs.

📋 Copy prompt for your agent 
CI on my pull request is failing. Help me find and fix the root cause of each failing job below — they were flagged as caused by changes in this PR, so focus on the diff. For each job, explain the failure and propose a fix.

Branch: pagebroker-checkpoint

Pre Merge | pre-commit
Commit: 9caf7096937a0b898009fae976516d84f88f0066
Error (code / build):
Pre-commit hook made changes in file formatting and syntax, which were not committed. Job exited with code 1.
CI job: https://github.com/ai-dynamo/dynamo/actions/runs/31950891821/job/95174073089

Pre Merge | snapshot
Commit: 9caf7096937a0b898009fae976516d84f88f0066
Error (code / build):
Linker error: undefined references to several functions in pagebroker causing the build to fail during testing.
CI job: https://github.com/ai-dynamo/dynamo/actions/runs/31950891821/job/95174100006

ℹ️ Info

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 50.82% (+3.26%)

Useful? React with 👍 / 👎

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

@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch 2 times, most recently from e732f37 to 76a3a17 Compare August 13, 2026 07:08
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from 76a3a17 to 1c9f438 Compare August 13, 2026 07:35
@dfeigin-nv dfeigin-nv mentioned this pull request Aug 13, 2026
Closed
hhzhang16
hhzhang16 reviewed Aug 14, 2026
View reviewed changes
Comment thread deploy/snapshot/internal/pagebroker/client.go
Comment on lines +45 to +53
func (c Client) Commit(ctx context.Context, transactionID string) error {
for {
response, err := c.request(ctx, transactionID, &Request_Commit{Commit: &CommitRequest{}})
if err == nil {
if response.GetCommitComplete() != nil {
return nil
}
return fmt.Errorf("unexpected PageBroker commit response")
}

@hhzhang16 hhzhang16 Aug 14, 2026

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.

Your MR description says that commit retries are bounded but there's no bounding here

@hhzhang16 hhzhang16 Aug 14, 2026

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.

Also there's retry work + tests in #13172 , might be better suited in this MR

@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from 1c9f438 to f5595aa Compare August 16, 2026 08:45
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch 2 times, most recently from 0527c2b to bb7c4f4 Compare August 16, 2026 10:34
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from bb7c4f4 to 9454fe2 Compare August 16, 2026 11:33
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from 9454fe2 to a6ee0f8 Compare August 16, 2026 13:29
@dfeigin-nv
dfeigin-nv force-pushed the pagebroker-checkpoint branch from a6ee0f8 to 9caf709 Compare August 16, 2026 13:48
@dfeigin-nv dfeigin-nv mentioned this pull request Aug 20, 2026
Open
@dfeigin-nv

dfeigin-nv commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Moved to ai-dynamo/snapshot#86.

@dfeigin-nv dfeigin-nv closed this Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hhzhang16 hhzhang16 hhzhang16 left review comments
@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments
+1 more reviewer
@dynamo-review-agent dynamo-review-agent[bot] dynamo-review-agent[bot] approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feat size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dfeigin-nv @hhzhang16