Skip to content

docs: document CUDA CustomStorage configuration - #120

Open
hhzhang16 wants to merge 2 commits into
ozb/alpha-docsfrom
hannahz/dgh-1458-document-snapshot-customstorage
Open

docs: document CUDA CustomStorage configuration#120
hhzhang16 wants to merge 2 commits into
ozb/alpha-docsfrom
hannahz/dgh-1458-document-snapshot-customstorage

Conversation

@hhzhang16

Copy link
Copy Markdown

Summary

Adds documentation for configuring and validating Snapshot's opt-in CUDA CustomStorage path (the behavior implemented in #109 + #96). The guide is linked from the main README, chart documentation, storage documentation, troubleshooting, and the guide index.

Review focus

Whether the CUDA capability requirements, Helm values, artifact compatibility, and current limitations are clear and accurate for someone enabling this for the first time.

Validation

  • Cross-checked the values, defaults, capability fields, and failure behavior against feat: add the gated local CustomStorage POSIX path #96.
  • Helm lint and render passed with the documented posix configuration.
  • All newly added links and anchors resolve.
  • git diff --check passes.

Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c857e048-938c-4796-8476-fa412efb28ef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment thread README.md Outdated
- The [Helm](https://helm.sh/docs/intro/install) CLI
- A cluster that permits privileged pods for the node agent — see [Security](docs/operations/security.md)

The default CUDA checkpoint path works with the general driver prerequisite above. Enabling [CUDA CustomStorage](docs/guides/custom-storage.md) additionally requires a driver that exports the CUDA 13.4 CustomStorage API; CUDA 13.x compatibility alone is not a capability check.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

CUDA 13.4 is not GA

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The CustomStorage path is an explicit opt-in using the CUDA 13.4 Developer Preview API, while the default remains the existing driver-managed path. The guide calls this out

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The helper checks cuCheckpointOperationComplete at runtime and rejects posix before mutating the workload when it isn’t available

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Which CUDA driver supports CUDA 13.4? I believe there is a support matrix of GPU operator + CUDA driver + CUDA version

The missing piece for me is which CUDA driver does customers needs to install

Comment thread charts/snapshot/README.md Outdated
| `seccomp.deploy` | Deploy the CRIU seccomp profile ConfigMap and init container. Use this field name; `seccomp.enabled` is not a chart value | `true` |
| `runtime.type` | CRI backend: `containerd` or `crio` | `containerd` |
| `runtime.socketPath` | CRI socket (empty = default for `runtime.type`) | `""` |
| `config.cudaCheckpoint.storageMode` | Storage policy for newly created CUDA checkpoints: `legacy` or `posix` | `legacy` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

move config.cudaCheckpoint -> storage.gpu.engine

storageMode -> backend [criu | nixl ] later we will also have

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

They are not interchangeable backend values imo -- CRIU is the process checkpoint engine, while NIXL POSIX is the CUDA artifact transfer backend.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I’m open to reorganizing the config hierarchy, but the accurate values would be closer to driver-managed | nixl-posix, not criu | nixl. Changing it would also require updating #96 rather than only renaming it in the docs

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I do want to change the hierarchy, of course not only in the docs but that's my reference as PM for these interface changes.
regarding values: @dfeigin-nv WDYT?

Comment thread charts/snapshot/README.md Outdated
| `config.cudaCheckpoint.storageMode` | Storage policy for newly created CUDA checkpoints: `legacy` or `posix` | `legacy` |
| `config.cudaCheckpoint.transferBufferCount` | Pinned transfer slots per active CUDA device | `4` |
| `config.cudaCheckpoint.transferChunkBytes` | Bytes per pinned transfer slot | `67108864` |
| `config.cudaCheckpoint.daemon.maxOperationSeconds` | Cooperative watchdog for one CUDA helper operation | `3600` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think "Cooperative watchdog for one CUDA helper operation" is unclear from a user perspective and also I don't get it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Reworded

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's the maximum time allowed for one CUDA checkpoint or restore helper request

```yaml
config:
cudaCheckpoint:
storageMode: posix

@ozbarshalom ozbarshalom Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure the full example is required here only to enable NIXL

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I included the transfer and resource values to show the operational defaults but I can reduce this to the minimal setting so it's not as scary/complicated-looking

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated so that the enablement example now contains only the required storageMode: posix setting. The tuning and resource options remain in the reference table.

Comment thread docs/guides/custom-storage.md Outdated
| `config.cudaCheckpoint.transferChunkBytes` | Bytes in each pinned transfer slot | `67108864` (64 MiB) | 1-256 MiB, 4096-byte aligned |
| `config.cudaCheckpoint.daemon.maxOperationSeconds` | Cooperative transfer watchdog for one helper operation | `3600` | `1`-`3600` |
| `config.cudaCheckpoint.daemon.resources` | CPU and memory requests and limits for the helper container | See the example above | Kubernetes resource values |
| `config.restore.restoreTimeoutSeconds` | Overall restore budget in the Snapshot agent | `8100` | Positive integer |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is it in the custom storage guide?

Comment thread docs/guides/custom-storage.md Outdated
@@ -0,0 +1,156 @@
# Use CUDA CustomStorage

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

custom storage engine

Comment thread docs/guides/custom-storage.md Outdated

The Snapshot agent image may be built against CUDA 13.0 headers because the helper dynamically resolves the public CUDA 13.4 entry point. The node driver, not the image's toolkit version, determines whether CustomStorage is available at runtime.

## Enable CustomStorage

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Customers don't "enable custom storage" they can modify the IO engine to use NIXL instead of CRIU.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think this is the same layer distinction as one of the above comments. NIXL does not replace CRIU here — CRIU still captures and restores the process state. Selecting posix changes the CUDA path from driver-managed checkpoint storage to the CUDA 13.4 CustomStorage API, with NIXL POSIX moving the resulting extents.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From a user perspective, customStorage is another storage rather than the existing supported storage which is local PVC. This is not the case. I do understand that for the CUDA C/R it's a custom storage backend / engine but we should slightly abstract it from Snapshot users. (naming wise)

@dfeigin-nv WDYT?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the naming should be something clearer than custom storage. Custom Storage is more of an implementation detail for us, not something the user should actively choose

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

only after criu finishes the IO does the driver-managed checkpoint storage start, so legacy | criu are both correct. What is most clear to the user should be used.

Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
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.

3 participants