fix: report all scalar metrics for each queue

[hajnalmt]

What type of PR is this?

Fixing scalar metrics handling

What this PR does / why we need it:

Previously, if a scalar resource was no longer handled in a queue (e.g., after the last job holding it finished, or the cluster didn't have that scalar resource anymore), the metric would retain its old value and not be zeroed out. This lead to stale and misleading metrics, for example if a scalar metric is not present in the cluster anymore, the last not 0 integer value was kept in queue_real_capacity_scalar_resources since nothing updated it to be zero, it just disappeared from scalarResources map[v1.ResourceName]float64 . Every type of queue scalar metric has this problem.

This change ensures that every known scalar resource for a queue is always reported, and any missing or unallocated resource is explicitly set to zero.

Metrics are properly cleaned up when a queue is deleted, preventing stale data and improving monitoring accuracy.

One consequence of this is that if a scalar resource disappears from the cluster, then it's metric will be zeroed and not deleted.
I conside this the right approach as it avoids "flapping" metrics that appear/disappear, which can confuse dashboards and alerting.

Which issue(s) this PR fixes:

Fixes: #4529

Special notes for your reviewer:

Instead of somehow deleting the scalar resource metric that's no longer used. I decided that zeroing it out is the right approach since it provides a cleaner interface, we do this with cpu and memory. I think we should do this here too.

I have introduced a knownScalarResources map which holds the known scalar resources for each queue. With this we can zero out the no longer used resources.

Does this PR introduce a user-facing change?

Queue scalar metrics are now reported on each resource. This change prevents stale scalar metrics reported by the scheduler.

[gemini-code-assist]

Summary of Changes

Hello @hajnalmt, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an issue where scalar resource metrics for queues could become stale, displaying outdated values for resources no longer in use or present in the cluster. The changes introduce a mechanism to actively track and zero out these missing metrics, ensuring that monitoring data accurately reflects the current state of queue resources. This significantly improves the reliability and interpretability of resource usage metrics within the scheduler.

Highlights

• Stale Scalar Metrics Resolution: Addresses a critical issue where scalar resource metrics for queues would retain old values even after the resources were no longer present, leading to misleading data.
• Introduction of knownScalarResources: A new internal map knownScalarResources is introduced to track all scalar resources associated with each queue, enabling explicit zeroing of metrics for resources that are no longer reported.
• Centralized Metric Update Logic: A new helper function, updateScalarResourceMetrics, centralizes the logic for updating scalar resource metrics, ensuring that any resource not currently reported is explicitly set to zero.
• Enhanced Metric Cleanup: The DeleteQueueMetrics function now also cleans up the knownScalarResources map when a queue is deleted, preventing memory leaks and ensuring complete metric removal.
• New Test Coverage: A new test file queue_scalar_test.go has been added to specifically verify the correct zeroing and cleanup behavior of scalar resource metrics.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly addresses the issue of stale scalar metrics by tracking known resources and zeroing them out when they are no longer present. The introduction of the updateScalarResourceMetrics helper function is a good refactoring. However, I've identified a critical race condition due to unsynchronized access to the new global knownScalarResources map. I've provided suggestions to add a mutex to protect this map. Additionally, the new test case in queue_scalar_test.go is not isolated and depends on global state, which can lead to flaky tests. I've suggested a way to reset the state at the beginning of the test to make it more robust. Please address these points to ensure thread safety and test reliability.

[hajnalmt]

I fixed the missing license header and the test logic.
The test without the change (which also proves the problems presented):

=== RUN   TestUpdateScalarResourceMetrics_ZeroAndCleanup
    queue_scalar_test.go:36: expected amd.com/gpu to be 0 after missing in update, got 10
    queue_scalar_test.go:42: expected nvidia.com/gpu to be 0 after nil update, got 3
    queue_scalar_test.go:45: expected amd.com/gpu to be 0 after nil update, got 10
--- FAIL: TestUpdateScalarResourceMetrics_ZeroAndCleanup (0.00s)
=== RUN   TestQueueResourceMetric
--- PASS: TestQueueResourceMetric (0.02s)
FAIL
FAIL    volcano.sh/volcano/pkg/scheduler/metrics        0.043s
FAIL

The tests now:

=== RUN   TestUpdateScalarResourceMetrics_ZeroAndCleanup
--- PASS: TestUpdateScalarResourceMetrics_ZeroAndCleanup (0.00s)
=== RUN   TestQueueResourceMetric
--- PASS: TestQueueResourceMetric (0.02s)
PASS
ok      volcano.sh/volcano/pkg/scheduler/metrics        (cached)

[hajnalmt]

@JesseStutler @Monokaix
Can you help me reviewing this? 😊 Will this implementation be okay?

[hajnalmt]

Rebased the PR.
Please check @JesseStutler and @Monokaix !

[hajnalmt]

/assign @kingeasternsun

[JesseStutler]

/cc

[JesseStutler]

/lgtm

[Monokaix]

/approve

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Monokaix

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

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/scheduler/OWNERS [Monokaix]

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

[hajnalmt]

Thank you for the review guys!

[JesseStutler]

Thank you for the review guys!

@hajnalmt Has your company already deployed Volcano in the production environment? If there's any need for communication(like features discussion, use discussion), we can contact each other anytime on Slack :)