docs(device): warn against Kubernetes quantity suffixes for device memory requests - #714
Open
veyron-kairo wants to merge 1 commit into
Open
docs(device): warn against Kubernetes quantity suffixes for device memory requests#714veyron-kairo wants to merge 1 commit into
veyron-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>
Contributor
|
[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
Comment on lines
+16
to
+20
| :::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.
Comment on lines
+13
to
+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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 .