Skip to content

[ML] No refresh on indexing DFA stats #53977

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.

Sign up for GitHub

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
merged 1 commit into from
Mar 24, 2020
+11 −7
Merged

[ML] No refresh on indexing DFA stats #53977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions ...l/src/main/java/org/elasticsearch/xpack/ml/dataframe/process/AnalyticsProcessManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.elasticsearch.xpack.ml.utils.persistence.ResultsPersisterService;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -178,7 +179,7 @@ private void processData(DataFrameAnalyticsTask task, ProcessContext processCont
processContext.setFailureReason(resultProcessor.getFailure());

refreshDest(config);
refreshStateIndex(config.getId());
refreshIndices(config.getId());
LOGGER.info("[{}] Result processor has completed", config.getId());
} catch (Exception e) {
if (task.isStopping()) {
Expand Down Expand Up @@ -316,12 +317,15 @@ private void refreshDest(DataFrameAnalyticsConfig config) {
() -> client.execute(RefreshAction.INSTANCE, new RefreshRequest(config.getDest().getIndex())).actionGet());
}

private void refreshStateIndex(String jobId) {
String indexName = AnomalyDetectorsIndex.jobStateIndexPattern();
LOGGER.debug("[{}] Refresh index {}", jobId, indexName);

RefreshRequest refreshRequest = new RefreshRequest(indexName);
private void refreshIndices(String jobId) {
Copy link
Contributor

@przemekwitek przemekwitek Mar 24, 2020

Choose a reason for hiding this comment

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

Would it make sense to also refresh dest index in this method?

Copy link
Contributor Author

@dimitris-athanasiou dimitris-athanasiou Mar 24, 2020

Choose a reason for hiding this comment

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

We need to use the config's headers to refresh the dest index which is why I kept it separate.

Copy link
Contributor

@przemekwitek przemekwitek Mar 24, 2020

Choose a reason for hiding this comment

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

Right, thanks for explanation.

RefreshRequest refreshRequest = new RefreshRequest(
AnomalyDetectorsIndex.jobStateIndexPattern(),
MlStatsIndex.indexPattern()
);
refreshRequest.indicesOptions(IndicesOptions.lenientExpandOpen());

LOGGER.debug("[{}] Refreshing indices {}", jobId, Arrays.toString(refreshRequest.indices()));
Copy link
Member

@benwtrent benwtrent Mar 23, 2020

Choose a reason for hiding this comment

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

Suggested change
LOGGER.debug("[{}] Refreshing indices {}", jobId, Arrays.toString(refreshRequest.indices()));
LOGGER.debug(() -> new ParameterizedMessage("[{}] Refreshing indices {}", jobId, Arrays.toString(refreshRequest.indices())));

I think the string is created eagerly. This is not strictly necessary unless the logging level is debug.

This is a minor thing as most of our logging does not take this into account.


try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(ML_ORIGIN)) {
client.admin().indices().refresh(refreshRequest).actionGet();
}
Expand Down
2 changes: 1 addition & 1 deletion ...ck/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/dataframe/stats/StatsPersister.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void persistWithRetry(ToXContentObject result, Function<String, String> d
MlStatsIndex.writeAlias(),
result,
new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")),
WriteRequest.RefreshPolicy.IMMEDIATE,
WriteRequest.RefreshPolicy.NONE,
docIdSupplier.apply(jobId),
() -> isCancelled == false,
errorMsg -> auditor.error(jobId,
Expand Down