Skip to content

Prevent duplicate GPU registration during health checks - #2054

Open
aryangorwade wants to merge 1 commit into
NVIDIA:mainfrom
aryangorwade:prevent-duplicate-gpu-registration
Open

aryangorwade wants to merge 1 commit into
NVIDIA:mainfrom
aryangorwade:prevent-duplicate-gpu-registration

Conversation

@aryangorwade

Copy link
Copy Markdown

Description

This PR removes redundant GPU health registration calls that can happen when multiple Device objects share the same parent GPU. It builds on changes made in #1930 and should be merged in afterwards.

Closes #2052 .

Checklist

  • No secrets, sensitive information, or unrelated changes
  • Lint checks passing (make lint)
  • Unit tests passing (make test)
  • Go mod artifacts in-sync (make check-modules)
  • Third-party notices in-sync (make check-third-party-notices)
  • [] Test cases are added for new code paths (test cases not possible without nvml mocks)

Testing

Verified via manual testing on a GPU node. Scenarios tested include unshared GPUs, time-sliced & MPS shared GPUs, multiple MIG instances and time-sliced MIG & mixed MIG. Plugin restart/config reload behavior was also tested.

Signed-off-by: Aryan Gorwade <agorwade@nvidia.com>

@kvalliyurnatt kvalliyurnatt 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.

LGTM. Is it worth adding a unit test for this ?

@aryangorwade

aryangorwade commented Sep 23, 2026 •

Copy link
Copy Markdown
Author

The only practical way I think we can test this is by mocking checkHealth's nvml event loop. This repo generally avoids doing that (the test file for this mocks nvml but it was recently written by an external contributor). It has its disadvantages like causing bugs like this to be present. If we do choose to write a test for checkHealth it should probably be an end-to-end test that checks every part of the loop. I think for now there's no point in adding a unit test only for this.

This branch has not been deployed

No deployments
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.

[Bug]: Duplicate GPU health checking registration

2 participants