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

Xapi thread classification - part 2 #6154

Conversation

GabrielBuica
Copy link
Contributor

@GabrielBuica GabrielBuica commented Dec 4, 2024

Adds more granurality in the xapi thread classification. Now, an
individual thread can be mapped to the following:

  • {xapi_cgroup}/internal/SM;
  • {xapi_cgroup}/internal/cli;
  • {xapi_cgroup}/external/intrapool;
  • {xapi_cgroup}/external/unauthenticated;
  • {xapi_cgroup}/external/authenticated/{identity};

where {identity} is a {auth_user_sid}/{user_agent}.
Both {auth_user_sid} and {user_agent} strings are sanitized when
making the identity through Identity.make, by allowing only
alphanumeric characters and a maximum length of 16 characters each.

This should allow for proper thread classification in xapi.

Note: Threads calling back into xapi from xenopsd are yet to be classified.

e.g. Cgroup hierarchy based on the new classification
image

BVT: 208947 & BST: 208972

@GabrielBuica GabrielBuica force-pushed the private/dbuica/feature/perf/tgroups-part-2 branch 2 times, most recently from f91bdb1 to 62a903a Compare December 5, 2024 16:49
@GabrielBuica GabrielBuica marked this pull request as ready for review December 5, 2024 17:25
ocaml/libs/tgroup/tgroup.ml Outdated Show resolved Hide resolved
ocaml/libs/tgroup/tgroup.ml Outdated Show resolved Hide resolved
ocaml/libs/tgroup/tgroup.ml Outdated Show resolved Hide resolved
@lindig
Copy link
Contributor

lindig commented Jan 7, 2025

@mg12 can we get a review? I don't like large PRs lingering here. And @GabrielBuica what about comments?

@GabrielBuica
Copy link
Contributor Author

@lindig I talked over with @mg12 and I was in the middle of addressing his comments.

Copy link
Member

@mg12 mg12 left a comment

Choose a reason for hiding this comment

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

it looks good to me, thanks

Adds more granurality in the xapi thread classification. Now, an
individual thread can be mapped to the following:

- {xapi_cgroup}/internal/SM;
- {xapi_cgroup}/internal/cli;
- {xapi_cgroup}/external/intrapool;
- {xapi_cgroup}/external/unauthenticated;
- {xapi_cgroup}/external/authenticated/{identity};

where {identity} is a {auth_user_sid}/{user_agent}.
Both {auth_user_sid} and {user_agent} strings are sanitized when
making the identity through `Identity.make`, by allowing only
alphanumenric characters and a maximum length of 16 characters each.

This is only the library change.

This should allow for proper thread classification in xapi.

Signed-off-by: Gabriel Buica <[email protected]>
Adds unit test for:
- the correct thread classification of `of_creator`;
- sanitation of `Identity.make`.

Signed-off-by: Gabriel Buica <[email protected]>
Classifies the threads at the time of session creation and inside
`do_dispatch`.

This ensures that new threads created by current session/request inherit
the propper classification.

Note: threads created by xenopsd calling back into xapi are yet to be
classified.

Signed-off-by: Gabriel Buica <[email protected]>
@GabrielBuica GabrielBuica force-pushed the private/dbuica/feature/perf/tgroups-part-2 branch from f29001c to 63391ba Compare January 8, 2025 08:41
@mg12 mg12 merged commit 6589d9a into xapi-project:feature/perf Jan 8, 2025
15 checks passed
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.

4 participants