docs(device): warn against Kubernetes quantity suffixes for device memory requests - #714
docs(device): warn against Kubernetes quantity suffixes for device memory requests#714veyron-kairo wants to merge 1 commit into
Conversation
…mory The per-device memory resources (nvidia.com/gpumem, hygon.com/dcumem, cambricon.com/mlu.smlu.vmemory, iluvatar.ai/<card>.vMem) take a plain integer. A Kubernetes quantity suffix such as 16Gi is parsed as its byte count, overflows HAMi's 32-bit memory field, and is silently truncated (often to 0). A zero request passes the scheduler's memory check unconditionally, so the pod is placed on a device that is already full and OOMs at runtime. Add a warning to each backend's memory doc explaining the integer format and the failure mode, correct hygon's "percentage size" wording (it uses fixed 1 MiB units, not a percentage), and align the hygon example comment. Applied to current docs and the v2.9.0 snapshot, in English and Chinese, per the AGENTS.md versioning workflow. The v2.9.0 sync also carries a couple of pre-existing minor fixes that current already had (versioning drift). Signed-off-by: Shridhar Panigrahi <198173519+veyron-kairo@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: veyron-kairo The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for project-hami ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe documentation updates define plain integer formats for device memory requests, clarify device-specific units, and describe overflow-related scheduler bypasses and runtime OOM failures. Version 2.9.0 guides also update a Helm image parameter and related examples. ChangesDevice memory guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
coderabbitai
Bot
added
the
kind/documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/userguide/cambricon-device/specify-device-memory-usage.md`:
- Around line 16-20: Generalize the warning in
docs/userguide/cambricon-device/specify-device-memory-usage.md lines 16-20 to
refer to the configured Cambricon *.smlu.vmemory resource rather than only
cambricon.com/mlu.smlu.vmemory, while preserving the plain-integer guidance.
Apply the same scope correction to the Chinese translation in
i18n/zh/docusaurus-plugin-content-docs/current/userguide/cambricon-device/specify-device-memory-usage.md
lines 16-20.
In
`@i18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/nvidia-device/specify-device-memory-usage.md`:
- Around line 13-20: Update the introductory sentence describing
nvidia.com/gpumem so each unit is consistently identified as 1 MiB, matching the
example and warning; leave the remaining guidance unchanged.
🪄 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: Pro Plus
Run ID: 7ec16d32-4aba-4852-b84a-e8597f2ecb3f
📒 Files selected for processing (16)
docs/userguide/cambricon-device/specify-device-memory-usage.mddocs/userguide/hygon-device/specify-device-memory-usage.mddocs/userguide/iluvatar-device/enable-iluvatar-gpu-sharing.mddocs/userguide/nvidia-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/cambricon-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/hygon-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/iluvatar-device/enable-iluvatar-gpu-sharing.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/nvidia-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/cambricon-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/hygon-device/specify-device-memory-usage.mdi18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/iluvatar-device/enable-iluvatar-gpu-sharing.mdi18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/nvidia-device/specify-device-memory-usage.mdversioned_docs/version-v2.9.0/userguide/cambricon-device/specify-device-memory-usage.mdversioned_docs/version-v2.9.0/userguide/hygon-device/specify-device-memory-usage.mdversioned_docs/version-v2.9.0/userguide/iluvatar-device/enable-iluvatar-gpu-sharing.mdversioned_docs/version-v2.9.0/userguide/nvidia-device/specify-device-memory-usage.md
| :::warning | ||
|
|
||
| Set `cambricon.com/mlu.smlu.vmemory` as a **plain integer** (here, a percentage from 1 to 100), not a Kubernetes quantity. A suffixed value such as `16Gi` is parsed as its byte count (about 17 billion), which overflows HAMi's 32-bit memory field and is silently truncated, often to `0`. A zero memory request passes the scheduler's memory check unconditionally, so the pod can be placed on an MLU that is already full and then fails with out-of-memory errors at runtime. Use the integer form shown above (for example `20`). | ||
|
|
||
| ::: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Apply the warning to plugin-specific Cambricon resource names.
The note at Line 24 says that the resource name can be cambricon.com/mlu370.smlu.vmemory or another type. The warning names only cambricon.com/mlu.smlu.vmemory. Refer to the configured *.smlu.vmemory resource so users of plugin-specific names receive the same integer-only guidance.
docs/userguide/cambricon-device/specify-device-memory-usage.md#L16-L20: Generalize the warning to the configured Cambricon memory resource.i18n/zh/docusaurus-plugin-content-docs/current/userguide/cambricon-device/specify-device-memory-usage.md#L16-L20: Apply the same scope correction in the Chinese translation.
📍 Affects 2 files
docs/userguide/cambricon-device/specify-device-memory-usage.md#L16-L20(this comment)i18n/zh/docusaurus-plugin-content-docs/current/userguide/cambricon-device/specify-device-memory-usage.md#L16-L20
🤖 Prompt for 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.
In `@docs/userguide/cambricon-device/specify-device-memory-usage.md` around lines
16 - 20, Generalize the warning in
docs/userguide/cambricon-device/specify-device-memory-usage.md lines 16-20 to
refer to the configured Cambricon *.smlu.vmemory resource rather than only
cambricon.com/mlu.smlu.vmemory, while preserving the plain-integer guidance.
Apply the same scope correction to the Chinese translation in
i18n/zh/docusaurus-plugin-content-docs/current/userguide/cambricon-device/specify-device-memory-usage.md
lines 16-20.
| nvidia.com/gpumem: 3000 # 每个 GPU 包含 3000 MiB 设备显存 | ||
| ``` | ||
|
|
||
| :::warning | ||
|
|
||
| 请将 `nvidia.com/gpumem` 设置为**纯整数**（以 MiB 为单位计数），不要使用 Kubernetes 数量单位。带单位的值（例如 `16Gi`）会被解析为字节数（约 170 亿），从而超出 HAMi 的 32 位显存字段范围并被静默截断，通常截断为 `0`。显存请求为 0 时会无条件通过调度器的显存检查，导致 Pod 被调度到显存已满的设备上，并在运行时发生显存不足（OOM）。请使用上面示例中的整数形式（例如 `3000`）。 | ||
|
|
||
| ::: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use MiB consistently.
Line 7 still says that each nvidia.com/gpumem unit equals 1M, but the updated example and warning use MiB. Change the introductory sentence to 1 MiB.
Proposed correction
-每个 `nvidia.com/gpumem` 单位等于 1M。
+每个 `nvidia.com/gpumem` 单位等于 1 MiB。🤖 Prompt for 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.
In
`@i18n/zh/docusaurus-plugin-content-docs/version-v2.9.0/userguide/nvidia-device/specify-device-memory-usage.md`
around lines 13 - 20, Update the introductory sentence describing
nvidia.com/gpumem so each unit is consistently identified as 1 MiB, matching the
example and warning; leave the remaining guidance unchanged.
mesutoezdil
left a comment
mesutoezdil
left a comment
There was a problem hiding this comment.
good catch and clear warning. rebase so the docs health ci runs on this pr.
|
|
||
| ```bash | ||
| helm install hami hami-charts/hami --set scheduler.kubeScheduler.imageTag={your kubernetes version} --set devices.iluvatar.enabled=true -n kube-system | ||
| helm install hami hami-charts/hami --set scheduler.kubeScheduler.image.tag={your kubernetes version} --set devices.iluvatar.enabled=true -n kube-system |
There was a problem hiding this comment.
same question as in #708: does the v2.9 chart use image.tag? if it still uses imageTag this breaks the snapshot.
| nvidia.com/gpumem: 3000 # Each GPU contains 3000 MiB device memory | ||
| ``` | ||
|
|
||
| :::warning |
There was a problem hiding this comment.
your own hami pr #2285 makes the plugin reject an overflowing value instead of truncating it. from which hami release does this page describe the old behaviour?
| :::warning | ||
|
|
||
| Set `nvidia.com/gpumem` as a **plain integer** (a count of MiB), not a Kubernetes quantity. A suffixed value such as `16Gi` is parsed as its byte count (about 17 billion), which overflows HAMi's 32-bit memory field and is silently truncated, often to `0`. A zero memory request passes the scheduler's memory check unconditionally, so the pod can be placed on a GPU that is already full and then fails with out-of-memory errors at runtime. Use the integer form shown above (for example `3000`). | ||
|
|
There was a problem hiding this comment.
nvidia.com/gpumem-percentage and nvidia.com/gpucores just below take a plain integer too. do they hit the same field, and should the warning cover them?
|
|
||
| :::warning | ||
|
|
||
| Set `nvidia.com/gpumem` as a **plain integer** (a count of MiB), not a Kubernetes quantity. A suffixed value such as `16Gi` is parsed as its byte count (about 17 billion), which overflows HAMi's 32-bit memory field and is silently truncated, often to `0`. A zero memory request passes the scheduler's memory check unconditionally, so the pod can be placed on a GPU that is already full and then fails with out-of-memory errors at runtime. Use the integer form shown above (for example `3000`). |
There was a problem hiding this comment.
confirmed the mechanism: with Memreq 0 and MemPercentagereq 101, memreq stays 0 at nvidia/device.go:918 and the check at 927 then passes on any card. the source even carries a //This incurs an issue comment there.
|
|
||
| :::warning | ||
|
|
||
| Set `iluvatar.ai/<card-type>.vMem` as a **plain integer** (a count of 256 MB units), not a Kubernetes quantity. A suffixed value such as `16Gi` is parsed as its byte count (about 17 billion), which overflows HAMi's 32-bit memory field and is silently truncated, often to `0`. A zero memory request passes the scheduler's memory check unconditionally, so the pod can be placed on a GPU that is already full and then fails with out-of-memory errors at runtime. Use the integer form shown above (for example `64`). |
There was a problem hiding this comment.
hami #2285 is titled reject int32-overflowing memory requests in iluvatar and mthreads, so on iluvatar this now errors instead of truncating. should the iluvatar wording differ from the nvidia, hygon and cambricon ones?
2 participants
What this does
Adds a warning to every backend's device-memory doc about a real footgun that currently has no documentation and silently causes out-of-memory failures.
The per-device memory resources —
nvidia.com/gpumem,hygon.com/dcumem,cambricon.com/mlu.smlu.vmemory,iluvatar.ai/<card-type>.vMem— each take a plain integer. If a user writes a Kubernetes quantity such as16Gi(a very natural thing to try for "16 GB of memory"), HAMi parses it as its byte count (~17 billion), which overflows HAMi's 32-bit memory field and is silently truncated, often to0. A zero memory request passes the scheduler's memory check unconditionally, so the pod is scheduled onto a device that is already full and then OOMs at runtime.I ran into this while fixing the underlying overflow in the code (Project-HAMi/HAMi#2285 / #2278): the device plugins now reject an overflowing value, but nothing tells users the correct format in the first place. This documents it.
Changes
:::warningto the nvidia, hygon, cambricon, and iluvatar memory docs explaining the integer format and the failure mode.Scope
English + Chinese, current (
/docs/next) and thev2.9.0released snapshot, following the "fixing the latest stable version" workflow inAGENTS.md. Syncing current →v2.9.0also carried two pre-existing minor fixes that current already had (the versioning drift noted in #660):3000m→3000 MiBandcan't→cannoton the nvidia page, and a helm-flag spelling on the iluvatar page.Checks
npx prettier --checkandnpx markdownlintpass on the changed files.Notes
A native Chinese speaker is welcome to refine the zh phrasing of the new warnings.
AI assistance disclosure
I used chat gpt to write the Chinese counter of this pr and read and understood everything translating it to English after writing .