[ML] Limit categorization memory usage#1167
Merged
droberts195 merged 3 commits intoelastic:masterfrom Apr 28, 2020
Merged
Conversation
Anomaly detection jobs have a model_memory_limit setting. This is supposed to restrict the amount of memory the job can use, however, in the past the limit only applied to anomaly detection and not to categorization. This change applies memory limiting to categorization as follows: - When a job is in hard_limit status no new categories will be created. The input document that could not be categorized is discarded as it cannot take part in anomaly detection without a category. The failed_category_count statistic is incremented each time this happens. - When a job is in soft_limit status, we stop recording examples for the category. Fixes elastic#1130
Author
|
retest |
tveasey
reviewed
Apr 28, 2020
Contributor
tveasey
left a comment
There was a problem hiding this comment.
This basically looks good. I have one question regarding difference in the way you're testing if allocations are allow.
lib/api/CFieldDataCategorizer.cc
Outdated
| m_NumRecordsHandled{0}, m_OutputFieldCategory{m_Overrides[MLCATEGORY_NAME]}, | ||
| m_MaxMatchingLength{0}, m_JsonOutputWriter{jsonOutputWriter}, | ||
| m_CategorizationFieldName{config.categorizationFieldName()}, | ||
| m_CategorizationFilter{}, m_PersistenceManager{persistenceManager} { |
Contributor
There was a problem hiding this comment.
nit: there doesn't seem to be any need to default initialise m_CategorizationFilter here. Also, how about initialising builtin types m_NumRecordsHandled, etc in the class body?
| //! A soft categorization failure means downstream components can continue, | ||
| //! by considering the input record to be in some "uncategorizable" | ||
| //! category. | ||
| static const int SOFT_CATEGORIZATION_FAILURE_ERROR; |
|
|
||
| bool CDataCategorizer::addExample(int categoryId, const std::string& example) { | ||
| // Don't add examples if we're memory-limited | ||
| if (m_Limits.resourceMonitor().getMemoryStatus() != model_t::E_MemoryStatusOk) { |
Contributor
There was a problem hiding this comment.
Is there any reason not to use this->areNewCategoriesAllowed() here? I couldn't think of one.
Author
There was a problem hiding this comment.
areNewCategoriesAllowed() returns false when the job is in hard_limit. But we stop adding examples as soon as the job goes to soft_limit. It's in the spec for memory limiting in #1130 and the PR description, but I will expand the comment in the code to say this.
droberts195
added a commit
to droberts195/ml-cpp
that referenced
this pull request
Apr 28, 2020
Anomaly detection jobs have a model_memory_limit setting. This is supposed to restrict the amount of memory the job can use, however, in the past the limit only applied to anomaly detection and not to categorization. This change applies memory limiting to categorization as follows: - When a job is in hard_limit status no new categories will be created. The input document that could not be categorized is discarded as it cannot take part in anomaly detection without a category. The failed_category_count statistic is incremented each time this happens. - When a job is in soft_limit status, we stop recording examples for the category. Backport of elastic#1167
droberts195
added a commit
that referenced
this pull request
Apr 28, 2020
Anomaly detection jobs have a model_memory_limit setting. This is supposed to restrict the amount of memory the job can use, however, in the past the limit only applied to anomaly detection and not to categorization. This change applies memory limiting to categorization as follows: - When a job is in hard_limit status no new categories will be created. The input document that could not be categorized is discarded as it cannot take part in anomaly detection without a category. The failed_category_count statistic is incremented each time this happens. - When a job is in soft_limit status, we stop recording examples for the category. Backport of #1167
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Anomaly detection jobs have a model_memory_limit setting. This is
supposed to restrict the amount of memory the job can use, however,
in the past the limit only applied to anomaly detection and not to
categorization.
This change applies memory limiting to categorization as follows:
created. The input document that could not be categorized is
discarded as it cannot take part in anomaly detection without a
category. The failed_category_count statistic is incremented
each time this happens.
for the category.
Fixes #1130