Skip to content

[ML] Track memory usage in CHierarchicalResultsNormalizer #2831

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

Merged

Conversation

valeriy42
Copy link
Contributor

@valeriy42 valeriy42 commented Mar 17, 2025

This PR makes CHierarchicalResultsNormalizer to track its memory usage.

Fixes #2244

@valeriy42 valeriy42 self-assigned this Mar 17, 2025
@valeriy42 valeriy42 marked this pull request as ready for review March 18, 2025 12:02
@valeriy42 valeriy42 requested a review from edsavage March 18, 2025 12:03
Copy link
Contributor

@edsavage edsavage left a comment

Choose a reason for hiding this comment

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

LGTM. Just the one comment regarding test case naming.

@valeriy42 valeriy42 requested a review from tveasey March 19, 2025 13:10
Copy link
Contributor

@tveasey tveasey left a comment

Choose a reason for hiding this comment

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

Nothing major. I think there are some small clean ups possible and I left a question. Otherwise, LGTM

Copy link
Contributor

@tveasey tveasey left a comment

Choose a reason for hiding this comment

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

LGTM (aside the build issue)

Copy link

@valeriy42 valeriy42 merged commit 51eda36 into elastic:main Mar 21, 2025
15 checks passed
@valeriy42
Copy link
Contributor Author

@wwang500 , this PR should lead to an increase in reported memory usage on jobs with a large number of partitions/influencers. Can you please verify this with the nightly runs?

valeriy42 added a commit to valeriy42/ml-cpp that referenced this pull request Mar 21, 2025
This PR makes CHierarchicalResultsNormalizer to track its memory usage.

Fixes elastic#2244
@wwang500
Copy link

Checked the test results from last two weeks, there are no step changes. large jobs or small jobs.

i.e., model_bytes from nightly jobs:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Memory usage of hierarchical results normalizer is not accounted for
4 participants