add rollover and delete history custom result index conditions #16
Conversation
jackiehanyang
requested review from
jmazanec15,
kaituo,
ohltyler and
ylwu-amzn
as code owners
| // We have to pass null for newIndexName in order to get Elastic to increment the index count. | ||
| RolloverRequest rollOverRequest = new RolloverRequest(resultIndexAlias, null); | ||
| // perform rollover and delete on AD custom result index alias | ||
| if (config.getCustomResultIndexOrAlias().startsWith(ADCommonName.CUSTOM_RESULT_INDEX_PREFIX)) { |
There was a problem hiding this comment.
I already did the check in getConfigsWithCustomResultIndexAlias. You don't need it.
There was a problem hiding this comment.
Removed the part that's checking if config.getCustomResultIndexOrAlias() starts with AD custom result index prefix or forecasting custom result index prefix
|
|
||
| // perform rollover and delete on Forecasting custom result index alias |
There was a problem hiding this comment.
You can remove forecasting code as the code is in base class. We will schedule two tasks: one for AD, and one for forecasting. The base case just abstracts out which it is run for (AD or forecasting). ADIndexManagement and ForecastIndexManagement will override rolloverAndDeleteHistoryIndex and rollover and delete their own indices.
There was a problem hiding this comment.
same as above, removed this part
| @@ -1234,35 +1242,113 @@ protected void rolloverAndDeleteHistoryIndex( | |||
| String rolloverIndexPattern, | |||
| IndexType resultIndex | |||
| ) { | |||
| if (!doesDefaultResultIndexExist()) { | |||
| return; | |||
|
|
|||
There was a problem hiding this comment.
You only roll over default indices when there is an error or there is no custom result indices.
There was a problem hiding this comment.
changed to perform rollover and delete on default result index first if default result index exist. Then perform rollover and delete custom result index
| proceedWithRolloverAndDelete(resultIndexAlias, rolloverRequest, allResultIndicesPattern, resultIndex, null); | ||
| } | ||
|
|
||
| private RolloverRequest buildRolloverRequest(String resultIndexAlias, String rolloverIndexPattern) { |
There was a problem hiding this comment.
for custom result index, we don't need addMaxIndexDocsCondition. For default result index, we only need addMaxIndexDocsCondition. In this way, if customers don't elect any ISM option from AD dashboard, we won't touch it and cx can decide on their own. I think you can put addMaxIndexDocsCondition outside and only do it if you are handling default result index.
There was a problem hiding this comment.
changed to only build addMaxIndexDocsCondition when performing rollover on default result index
| @@ -283,7 +289,7 @@ protected void choosePrimaryShards(CreateIndexRequest request, boolean hiddenInd | |||
| ); | |||
| } | |||
|
|
|||
| protected void deleteOldHistoryIndices(String indexPattern, TimeValue historyRetentionPeriod) { | |||
| protected void deleteOldHistoryIndices(String indexPattern, TimeValue historyRetentionPeriod, Integer customResultIndexTtl) { | |||
There was a problem hiding this comment.
This is running for default result indices. But we want to check each custom index's custom ttl and do it one by one. Also, since there can be a lot of custom indices and the method is async, you may want to run the delete in batches. Finish batch of delete before the other custom indices.
There was a problem hiding this comment.
Also, can we still keep current method signature?
void deleteOldHistoryIndices(String indexPattern, TimeValue historyRetentionPeriod)
If it is default index, you called it the old way. If it is custom index, you translate customResultIndexTtl of type integer to historyRetentionPeriod of type TimeValue.
There was a problem hiding this comment.
kept current method signature.
Not sure what you mean by run the delete in batches. Right now in deleteOldHistoryIndices, it seems like we already collecting candidates indices for deletion into a list, then perform deletion. Let's sync offline about this
| config.getCustomResultIndexOrAlias(), | ||
| rolloverRequest, | ||
| getAllCustomResultIndexPattern(config.getCustomResultIndexOrAlias()), | ||
| (IndexType) ADIndex.CUSTOM_RESULT, |
There was a problem hiding this comment.
You can make ADIndex.CUSTOM_RESULT as an input of the calling method and let the sublcass to pass in ADIndex.CUSTOM_RESULT or ForecastIndex.CUSTOM_RESULT.
There was a problem hiding this comment.
updated this part to not differentiate AD and Forecasting.
jackiehanyang
force-pushed
the
jackieForecasting18_3
branch
10 times, most recently
from
2730df5
to
d5ba7f5
Compare
Signed-off-by: Jackie Han <[email protected]>
jackiehanyang
force-pushed
the
jackieForecasting18_3
branch
from
fac3bbd
to
864de68
Compare
Signed-off-by: Jackie Han <[email protected]>
jackiehanyang
force-pushed
the
jackieForecasting18_3
branch
from
7a98581
to
3a07ec8
Compare
Signed-off-by: Jackie Han <[email protected]>
add rollover conditions (size & age) and delete history index condition (ttl) on custom result index