[ML] Ensure bulk requests are not over memory limit#60219
Merged
dimitris-athanasiou merged 5 commits intoelastic:masterfrom Jul 28, 2020
Merged
Conversation
Data frame analytics jobs that work with very large datasets may produce bulk requests that are over the memory limit for indexing. This commit adds a helper class that bundles index requests in bulk requests that steer away from the memory limit. We then use this class both from the results joiner and the inference runner ensuring data frame analytics jobs do not generate bulk requests that are too large. Note the limit was implemented in elastic#58885.
Collaborator
|
Pinging @elastic/ml-core (:ml) |
benwtrent
approved these changes
Jul 27, 2020
Member
benwtrent
left a comment
There was a problem hiding this comment.
We should try to use this type of logic in the anomaly job result bulk indexer as well.
| modelLoadingService = mock(ModelLoadingService.class); | ||
| processManager = new AnalyticsProcessManager(client, executorServiceForJob, executorServiceForProcess, processFactory, auditor, | ||
| trainedModelProvider, modelLoadingService, resultsPersisterService, 1); | ||
| processManager = new AnalyticsProcessManager(Settings.builder().build(), client, executorServiceForJob, executorServiceForProcess, |
There was a problem hiding this comment.
Would Settings.EMPTY work here as well?
Contributor
Author
There was a problem hiding this comment.
I wanted to change those and forgot. Thanks for spotting!
|
|
||
| verifyNoMoreInteractions(resultsPersisterService); | ||
| List<BulkRequest> capturedBulkRequests = bulkRequestCaptor.getAllValues(); | ||
| assertThat(capturedBulkRequests.size(), equalTo(1)); |
There was a problem hiding this comment.
hasSize matcher could be used here instead.
|
|
||
| public class MlBulkIndexerTests extends ESTestCase { | ||
|
|
||
| private List<BulkRequest> executedBulkRequests = new ArrayList<>(); |
Contributor
Author
There was a problem hiding this comment.
This is getting reinitialised for each test so it cannot be final.
| * that do exceed a 1000 operations or half the available memory | ||
| * limit for indexing. | ||
| */ | ||
| public class MlBulkIndexer implements AutoCloseable { |
There was a problem hiding this comment.
There is no ML-specific code in this class. Would it make sense to rename it to BulkIndexer?
Contributor
Author
There was a problem hiding this comment.
I wanted to avoid calling it plainly BulkIndexer as it sounds too much like a basic ES component. I decided to prefix with Ml to indicate this is a utility designed to serve purposes within the ML plugin. If we end up using it in other places then we shall move it and rename it accordingly.
Contributor
Author
|
@elasticmachine update branch |
dimitris-athanasiou
added a commit
to dimitris-athanasiou/elasticsearch
that referenced
this pull request
Jul 28, 2020
Data frame analytics jobs that work with very large datasets may produce bulk requests that are over the memory limit for indexing. This commit adds a helper class that bundles index requests in bulk requests that steer away from the memory limit. We then use this class both from the results joiner and the inference runner ensuring data frame analytics jobs do not generate bulk requests that are too large. Note the limit was implemented in elastic#58885. Backport of elastic#60219
dimitris-athanasiou
added a commit
that referenced
this pull request
Jul 28, 2020
…0283) Data frame analytics jobs that work with very large datasets may produce bulk requests that are over the memory limit for indexing. This commit adds a helper class that bundles index requests in bulk requests that steer away from the memory limit. We then use this class both from the results joiner and the inference runner ensuring data frame analytics jobs do not generate bulk requests that are too large. Note the limit was implemented in #58885. Backport of #60219
dimitris-athanasiou
added a commit
that referenced
this pull request
Jul 28, 2020
…0284) Data frame analytics jobs that work with very large datasets may produce bulk requests that are over the memory limit for indexing. This commit adds a helper class that bundles index requests in bulk requests that steer away from the memory limit. We then use this class both from the results joiner and the inference runner ensuring data frame analytics jobs do not generate bulk requests that are too large. Note the limit was implemented in #58885. Backport of #60219
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
5 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.
Data frame analytics jobs that work with very large datasets
may produce bulk requests that are over the memory limit
for indexing. This commit adds a helper class that bundles
index requests in bulk requests that steer away from the
memory limit. We then use this class both from the results
joiner and the inference runner ensuring data frame analytics
jobs do not generate bulk requests that are too large.
Note the limit was implemented in #58885.