Add required config changes #264
Conversation
This PR adds config changes required for multi-entity detectors. Testing done: 1. Manually tested nothing breaks with these changes.
Codecov Report
@@ Coverage Diff @@
## master #264 +/- ##
=========================================
Coverage 73.01% 73.01%
Complexity 1461 1461
=========================================
Files 164 164
Lines 6834 6834
Branches 527 527
=========================================
Hits 4990 4990
Misses 1594 1594
Partials 250 250
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| memoryTracker | ||
| ); | ||
|
|
||
| // EntityCache cache = new LRUCache( |
There was a problem hiding this comment.
Remove these commented code ?
| // cache related | ||
| public static final int DEDICATED_CACHE_SIZE = 10; | ||
|
|
||
| // We only keep priority (4 bytes float) in inactive cache. 1 million priorities |
There was a problem hiding this comment.
So we store active entity's priority in inactive entity's cache ? If that's the case, why not name it as MAX_ENTITIES ?
There was a problem hiding this comment.
we don't do that. Active entities' states has both models and priority, while inactive entities' states has only priority.
There was a problem hiding this comment.
Got it, misunderstanding, thought all priority of active&inactive entity will be stored in inactive cache.
| public static int MAX_BULK_CHECKPOINT_SIZE = 1000; | ||
|
|
||
| // number of bulk checkpoints per second | ||
| public static double CHECKPOINT_BULK_PER_SECOND = 0.02; |
There was a problem hiding this comment.
Can execute 1 bulk index checkpoint every 50 seconds?
There was a problem hiding this comment.
yeah, 1/60 is not a finite decimal representation. It is fine.
| // take up 4 MB. | ||
| public static final int MAX_INACTIVE_ENTITIES = 1_000_000; | ||
|
|
||
| // TODO: check how much does 1 million insertion costs in memory |
There was a problem hiding this comment.
Do we still have to do a performance test get the accurate number?
There was a problem hiding this comment.
seems roughly 1 MB per 1 million from Weicong's jmap data.
This PR is a conglomerate of the following PRs. opendistro-for-elasticsearch#247 opendistro-for-elasticsearch#249 opendistro-for-elasticsearch#250 opendistro-for-elasticsearch#252 opendistro-for-elasticsearch#253 opendistro-for-elasticsearch#256 opendistro-for-elasticsearch#257 opendistro-for-elasticsearch#258 opendistro-for-elasticsearch#259 opendistro-for-elasticsearch#260 opendistro-for-elasticsearch#261 opendistro-for-elasticsearch#262 opendistro-for-elasticsearch#263 opendistro-for-elasticsearch#264 opendistro-for-elasticsearch#265 opendistro-for-elasticsearch#266 opendistro-for-elasticsearch#267 opendistro-for-elasticsearch#268 opendistro-for-elasticsearch#269 This spreadsheet contains the mappings from files to PR number: https://quip-amazon.com/DiHkAmz9oSLu/HC-PR Testing done: 1. Add unit tests except four classes (excluded in build.gradle). Will add them in the later PR. 2. Manual testing passes.
* Add support filtering the data by one categorical variable This PR is a conglomerate of the following PRs. #247 #249 #250 #252 #253 #256 #257 #258 #259 #260 #261 #262 #263 #264 #265 #266 #267 #268 #269 This spreadsheet contains the mappings from files to PR number: https://quip-amazon.com/DiHkAmz9oSLu/HC-PR Testing done: 1. Add unit tests except four classes (excluded in build.gradle). Will add them in the later PR. 2. Manual testing passes.
Note: since there are a lot of dependencies, I only list the main class and test code to save reviewers' time. The build will fail due to missing dependencies. I will use that PR just for review. will not merge it. Will have a big one in the end and merge once after all review PRs get approved.
Issue #, if available:
Description of changes:
This PR adds config changes required for multi-entity detectors.
Testing done:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.