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

Provide access to resource usage for processes and nodes #335

Open
wants to merge 1 commit into
base: v4
Choose a base branch
from

Conversation

rhc54
Copy link
Member

@rhc54 rhc54 commented Jan 31, 2021

Define two attributes by which applications and/or tools can request
resource usage of processes and nodes. Define a structure for each
case to contain the information, and associated macros for constructing
and destructing those structures.

Signed-off-by: Ralph Castain [email protected]

Define two attributes by which applications and/or tools can request
resource usage of processes and nodes. Define a structure for each
case to contain the information, and associated macros for constructing
and destructing those structures.

Signed-off-by: Ralph Castain <[email protected]>
@rhc54 rhc54 added the RFC Request for Comment label Jan 31, 2021
@rhc54 rhc54 added this to the PMIx v4.1 Standard milestone Jan 31, 2021
@rhc54 rhc54 self-assigned this Jan 31, 2021
@rhc54
Copy link
Member Author

rhc54 commented Jan 31, 2021

Please use emoji reactions ON THIS COMMENT to indicate your position on this proposal.

  • You do not need to vote on every proposal
  • If you have no opinion, don't vote - that is also useful data
  • If you've already commented on this issue, please still vote so
    we know your current thoughts
  • Not all proposals solve exactly the same problem, so we may end
    up accepting proposals that appear to have some overlap
    This is not a binding majority-rule vote, but it will be a very
    significant input into the corresponding ASC decision.

Here are the meanings for the emojis:

  • Hooray or Rocket: I support this so strongly that I
    want to be an advocate for it
  • Heart: I think this is an ideal solution
  • Thumbs up: I'd be happy with this solution
  • Confused: I'd rather we not do this, but I can tolerate it
  • Thumbs down: I'd be actively unhappy, and may even consider
    other technologies instead
    If you want to explain in more detail, feel free to add another
    comment, but please also vote on this comment.

Copy link
Member

@jjhursey jjhursey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor wording suggestions.

If no node is specified, then
results for all nodes hosting processes within the job of the requestor
will be returned in a \refstruct{pmix_data_array_t} of \refstruct{pmix_node_stats_t} structures.
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the PMIX_PROC_RESOURCE_USAGE should we point out that:

This attribute can be used with qualifiers such as \refattr{PMIX_SESSION_ID} to identify the session of the namespace whose statistics are being requested.

pmix_proc_t proc;
pid_t pid;
char *cmd;
/* process stats */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a link to more descriptions on each of these stats? We take them from Linux so a pointer to a good manpage description would be helpful to someone new to these fields.

@jjhursey
Copy link
Member

We should add a Python type for these.

@jjhursey
Copy link
Member

jjhursey commented Apr 7, 2021

This PR adds some new structures. This PR is on the provisional track.

There are some minor notes above that need to be addressed before the presentation.

@jjhursey
Copy link
Member

jjhursey commented Apr 9, 2021

We need to update the pmix_value_t structure with these two :

        pmix_proc_stats_t *pstats;
        pmix_node_stats_t *ndstats;

I see quite a few other fields that need to be added to the pmix_value_t that are in OpenPMIx v4. I'll add those in a separate PR if you can add the above two to this PR.

@jjhursey
Copy link
Member

jjhursey commented Apr 9, 2021

Implementation Notes:

@jjhursey jjhursey added the Eligible Eligible for consideration by ASC label Apr 15, 2021
@jjhursey
Copy link
Member

Notes from 2Q 2021 ASC Meeting (day 1)

  • Comments need to be addressed. If they can be addressed before Day 2 then we can have a Revision Exception vote.
    • this comment regarding the need for documentation of the various fields in the pmix_proc_stats_t. Should also document the fields of pmix_node_stats_t (what if the value is not available - should it be set to 0?).
    • this comment implementation requirement for PMIX_PROC_RESOURCE_USAGE and PMIX_NODE_RESOURCE_USAGE missing in OpenPMIx.
    • this comment missing pmix_proc_stats_t and pmix_node_stats_t fields in the pmix_value_t
  • Question regarding the choice of the data type for fields in pmix_node_stats_t - why float and not size_t/double/...? Is it better to have an accessor function instead of an explicit data structure?
  • Question if there are users of this interface at the moment.

@jjhursey jjhursey added the Pushed Back Pushed back by ASC label May 20, 2021
@jjhursey
Copy link
Member

PMIx ASC 2Q 2021: May 11 & May 13

  • This PR has been pushed back for the reasons summarized in this comment
  • Once those comments have been addressed this PR can be brought back for presentation.

@jjhursey jjhursey removed the Eligible Eligible for consideration by ASC label May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pushed Back Pushed back by ASC RFC Request for Comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants