[Core][GPU fraction][3/n] Encapsulate NodeResources::available behind wrapper methods

[dancingactor]

Description

Context

• This PR is a preparatory refactor for the final goal to change available type from NodeResourceSet (scalar) to NodeResourceInstanceSet (per-instance vector) for accurate GPU fragmentation detection([core] Fix GPU fractional scheduling by tracking per-instance availability #62005)
• To safely and gradually migrate this big change, we are leveraging branch by abstraction achieved via runtime polymorphism. We introduces a base case and lets both the old class and the new class inherit from it, and change the codebase to interact with the base class pointer. Then, we can use a config flag to toggle what the base class pointer actually points to (the old class or the new class) to control which logic the codebase uses

What This PR Does

• To achieve runtime polymorphism, this PR encapsulates the direct access to the available field behind wrapper methods so that we can include the wrapper methods in the base case

Note: This PR contains no logic changes, it only wraps original direct access calls with wrapper methods

Introduced methods in NodeResources

Method	Purpose
GetAvailableSum(ResourceID)	Get the scalar available amount for a resource
GetAvailableResourceIds()	Get the set of resource IDs that have explicit available entries
GetAvailableResourceMap()	Return available resources as a name->value map
SubtractAvailable(ResourceSet)	Subtract resources from available, clamping each entry to 0
SetAvailableResource(ResourceID, FixedPoint)	Set a single resource's available to an explicit scalar value
SetAvailable(NodeResourceSet)	Replace the entire available field from a NodeResourceSet
HasAvailableResource(ResourceID)	Returns true if the resource has an explicit available entry
GetAvailable()	Read-only access to the entire available resource set
TakeAvailable()	Transfer ownership of the available resource set (move out)

Related PRs

Related to #62005

[gemini-code-assist]

Code Review

This pull request encapsulates the available resource set in the NodeResources class by adding getter and setter methods, updating the GCS and cluster resource managers accordingly. The review feedback suggests optimizing the SetAvailable method and its usage by employing std::move to prevent unnecessary copies of resource sets.

[Yicheng-Lu-llll]

Thanks so much for the PR! The code change LGTM.

Could you please update the PR description to explain why we need to first refactor to use wrapper methods?

[dancingactor]

Thanks for review! I have updated the PR description to clarify the context and the reason for this refactor

[Yicheng-Lu-llll]

lgtm. Thanks! cc @MengjinYan for review/merge.

[MengjinYan]

Overall looks good!

I have a couple of general questions:

• Wondering are you aiming to replace all the reference to available with the newly added base functions?
• After the change, the available field is still public so we cannot prevent future PRs from accessing it elsewhere. Should we make it private to complete the encapsulation?
• I understand that it is not necessary to create the functions for accessing total. At the same time, the access will be asymmetric which might confuse people. So wondering what's the end state of the available and total fields?

[dancingactor]

Really appreciate your feedbacks!

• Wondering are you aiming to replace all the reference to available with the newly added base functions?

Yes, the goal is to replace all direct references to NodeResources.available with these wrapper. Note that there's a really similiar class called NodeResourceInstances also have available field. That class is out of scope for this specific PR

• After the change, the available field is still public so we cannot prevent future PRs from accessing it elsewhere. Should we make it private to complete the encapsulation?

Yes, I will make it private. Thanks!

• I understand that it is not necessary to create the functions for accessing total. At the same time, the access will be asymmetric which might confuse people. So wondering what's the end state of the available and total fields?

I think we should encapsulate both the available and total fields. As you pointed out, total is not necessary right now, so I will add the changes for total in the future cleanup PR

[dancingactor]

@dayshah @edoakes Sorry for the accidental ping! A messy rebase temporarily triggered an automated review request. The history is clean now, so please feel free to ignore this PR.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 24f1f0f. Configure here.}

[dancingactor]

Hi @MengjinYan, I have applied the changes in 24f1f0f. PTAL

• Moved available field to private
• Modified all tests to use wrapper methods
• Updated remaining places of direct access to use wrapper methods.
• Introduced HasAvailableResource, GetAvailable and TakeAvailable methods

[dancingactor]

Hi @MengjinYan, gentle ping on this PR for review/merge whenever you have a moment. Thanks again!