Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and refine the resource usage API implementation #2101

Open
achimnol opened this issue May 3, 2024 · 0 comments
Open

Refactor and refine the resource usage API implementation #2101

achimnol opened this issue May 3, 2024 · 0 comments
Assignees
Labels
comp:manager Related to Manager component type:refactor Refactor codes or add tests.
Milestone

Comments

@achimnol
Copy link
Member

achimnol commented May 3, 2024

We have a new resource usage summarization API to get the usage by project/session/kernel for given periods in #962.

However, we need to refactor the implementation.

  • Avoid including unnecessary information in some outputs. For example, the per-project resource usage currently includes metadata about a session and kernel. Why??
    • In the code, all ProjectResourceUsage, SessionResourceUsage, and KernelResourceUsage inherits BaseResourceUsageGroup. (Here, the name "Group" is not related to the concept of projects.)
    • BaseResourceUsageGroup contains direct references to a project, a session, and a kernel object. All subclasses merge the result of to_json() method of the superclass, and this incurs unnecessary inclusion of garbage information.
    • Let's rewrite BaseResourceUsageGroup as a simpler abstract base class like ResourceUsageAggregator with just a few methods (no attributes) like get_total_usage().
    • Just refer this abstract base class when calculating the total usage.
  • General tips
    • When defining base classes and subclasses to share the type while differentiating the concrete implementations, do NOT hesitate to make empty classes or declaring just a few abstract methods. It's totally fine. Avoid putting too much subclass-specifics in the base classes.
  • Rename/expand some fields.
    • ResourceUsage.nfs: Currently it contains the list of host directory paths of mounted vfolders in the given resource set. "nfs" is a too implementation-specific name. We should relax the concept, like ResourceUsage.vfolder_host_paths.
      • I think we need to add ResourceUsage.volumes to include the set of vfolder hosts (storage volumes) used by the given resource set.
@achimnol achimnol added comp:manager Related to Manager component type:refactor Refactor codes or add tests. labels May 3, 2024
@achimnol achimnol added this to the 24.09 milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component type:refactor Refactor codes or add tests.
Projects
None yet
Development

No branches or pull requests

2 participants