fix: warn when device list exceeds kubelet gRPC limit

[mesutoezdil]

Fixes #2187 in Project-HAMi/HAMi.
gpuMemoryFactor decides how many memory devices the plugin registers per GPU. All of them go out in a single ListAndWatch response, and kubelet reads it with the default 4MB gRPC limit, so about 60000 devices. Above that the response is dropped and volcano.sh/vgpu-memory stays at 0 or keeps its last value, with nothing in the plugin log. An 80GB card produces 81920 devices at factor 1, so two cards already pass the limit. This logs the count and the factor to use, and documents the constraint.

Summary by CodeRabbit

• Documentation

  • Clarified that each GPU memory unit is registered as a separate device.
  • Added guidance on staying within kubelet’s effective per-node device-entry limits, including updated examples for 80 GB GPUs (factor 2) and nodes with multiple 80 GB cards.

• Bug Fixes

  • Added validation and warning/error messaging when GPU memory configuration is likely to exceed kubelet’s device-entry limit, helping prevent incomplete device listing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 761d89e7-bb4d-4e81-9b2b-1555b2d0b5e5

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0316a and c5eebe2.

📒 Files selected for processing (3)

• doc/design.md
• pkg/plugin/server.go
• pkg/plugin/server_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• doc/design.md
• pkg/plugin/server_test.go
• pkg/plugin/server.go

---

📝 Walkthrough

Walkthrough

The plugin validates generated GPU memory device counts against kubelet’s effective response limit, logs excess-entry errors once, adds threshold tests, and documents the per-node GPU memory constraint.

Changes

GPU memory entry validation

Layer / File(s)	Summary
Memory device entry limit check pkg/plugin/server.go, pkg/plugin/server_test.go, doc/design.md	Adds the entry-count validation helper, performs one-time validation during memory device generation, tests factor-dependent thresholds, and documents the GPU memory factor constraint.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• #140 — Addresses GPU memory unit registration and resulting device-entry scalability limits.

Suggested labels: enhancement

Suggested reviewers: sataqiu, archlitchi, dsfans2014

Sequence Diagram(s)

sequenceDiagram
  participant apiDevices
  participant checkDeviceEntries
  participant Logger
  apiDevices->>checkDeviceEntries: validate generated memory-device count
  checkDeviceEntries-->>apiDevices: return validation error or success
  apiDevices->>Logger: log an entry-limit warning once

Loading

Poem

I’m a small rabbit with ears held high,
Counting GPU devices as they multiply.
A factor keeps the entries neat,
While tests make every limit meet.
Hop, hop—clear docs complete!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 accurately summarizes the main change: adding a warning when the device list exceeds kubelet’s gRPC limit.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pkg/plugin/server_test.go (1)

254-260: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add an exact-boundary regression case.

The current failure case does not catch over-recommendation when count*factor is exactly divisible by the limit. Add an assertion that 120,000 entries at factor 1 recommends “at least 2”, not “at least 3”.

🤖 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 `@pkg/plugin/server_test.go` around lines 254 - 260, Extend
TestCheckDeviceEntries with an exact-boundary assertion for 120000 entries at
factor 1, verifying checkDeviceEntries returns an error containing “at least 2”
and does not over-recommend “at least 3”.

🤖 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 `@doc/design.md`:
- Line 18: Update the GPU-memory-factor guidance in the memory-unit registration
example to say the per-node value is at most 60000, reflecting the inclusive
limit. Correct the 80GB examples so one card requires factor 2 and two cards
require factor 3, without stating that factor 4 is required.

In `@pkg/plugin/server.go`:
- Around line 66-71: Update checkDeviceEntries to calculate the recommended
gpuMemoryFactor using ceiling division, avoiding the extra increment when the
scaled device count divides evenly by deviceEntryLimit. Add a regression case in
pkg/plugin/server_test.go around lines 254-260 for 120,000 entries at factor 1,
asserting the recommendation is factor 2.

---

Nitpick comments:
In `@pkg/plugin/server_test.go`:
- Around line 254-260: Extend TestCheckDeviceEntries with an exact-boundary
assertion for 120000 entries at factor 1, verifying checkDeviceEntries returns
an error containing “at least 2” and does not over-recommend “at least 3”.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e91bad0-ca67-4906-aafb-b331cd414574

📥 Commits

Reviewing files that changed from the base of the PR and between 6561f1c and 0c777b7.

📒 Files selected for processing (3)

• doc/design.md
• pkg/plugin/server.go
• pkg/plugin/server_test.go

[imantaba]

Turning that silent 4MB drop into a logged, actionable message is a real improvement. A few thoughts:

1. Validate once instead of on every ListAndWatch send. checkDeviceEntries runs inside apiDevices(), which ListAndWatch calls on the initial send and again on every health event. Since GPUMemoryFactor and the card's memory are static, the count can't change between sends, so the same error re-logs on each resend. Could we compute this once in Start() (before Serve()/Register()) so the operator sees it at boot and it isn't recomputed per stream? klog.Warning might also fit better than klog.Error, since the plugin keeps running and it's an operator-config hint.

2. Minor off-by-one in the suggested factor. factor*uint(count)/deviceEntryLimit + 1 over-recommends by one when count*factor is an exact multiple of the limit - e.g. 120000 MB at factor 1 suggests factor 3, but factor 2 yields exactly 60000, which checkDeviceEntries accepts. ceil avoids it: (factor*uint(count) + deviceEntryLimit - 1) / deviceEntryLimit. Rare in practice, but worth a boundary test alongside the existing ones.

3. (nit) The deviceEntryLimit = 60000 margin assumes ~66 B/entry; the memory IDs land around that, so it looks safe - maybe reference the 4MB defaultServerMaxReceiveMessageSize in the comment so the derivation stays traceable if the ID format changes.

[SataQiu]

It seems to be just a log warning, without altering the original behavior.
/lgtm

/cc @archlitchi

[mesutoezdil]

Turning that silent 4MB drop into a logged, actionable message is a real improvement. A few thoughts:

1. Validate once instead of on every ListAndWatch send. checkDeviceEntries runs inside apiDevices(), which ListAndWatch calls on the initial send and again on every health event. Since GPUMemoryFactor and the card's memory are static, the count can't change between sends, so the same error re-logs on each resend. Could we compute this once in Start() (before Serve()/Register()) so the operator sees it at boot and it isn't recomputed per stream? klog.Warning might also fit better than klog.Error, since the plugin keeps running and it's an operator-config hint.
2. Minor off-by-one in the suggested factor. factor*uint(count)/deviceEntryLimit + 1 over-recommends by one when count*factor is an exact multiple of the limit - e.g. 120000 MB at factor 1 suggests factor 3, but factor 2 yields exactly 60000, which checkDeviceEntries accepts. ceil avoids it: (factor*uint(count) + deviceEntryLimit - 1) / deviceEntryLimit. Rare in practice, but worth a boundary test alongside the existing ones.
3. (nit) The deviceEntryLimit = 60000 margin assumes ~66 B/entry; the memory IDs land around that, so it looks safe - maybe reference the 4MB defaultServerMaxReceiveMessageSize in the comment so the derivation stays traceable if the ID format changes.

Reminder: Answers must be written by human being. You can view the relevant rule here.
https://github.com/Project-HAMi/HAMi/blob/master/CONTRIBUTING.md#contribution-gates

[archlitchi]

/lgtm

[hami-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: archlitchi, mesutoezdil, SataQiu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [archlitchi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment