From b0830626892723f5bff8876c1c24bcb390cf51a9 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Wed, 24 May 2017 11:43:25 +0100 Subject: [PATCH] [ML] Add document type to ID (elastic/x-pack-elasticsearch#1525) * Add document type to ID * Delete v5.4 quantiles Original commit: elastic/x-pack-elasticsearch@d1f383b9720dbcc0b05a64f83f68cf44193d982e --- .../persistence/JobStorageDeletionTask.java | 18 +++++--- .../process/autodetect/state/DataCounts.java | 2 +- .../autodetect/state/ModelSnapshot.java | 8 +++- .../process/autodetect/state/Quantiles.java | 2 +- .../ml/integration/DeleteExpiredDataIT.java | 2 +- .../integration/MlDistributedFailureIT.java | 2 +- .../autodetect/state/DataCountsTests.java | 6 +++ .../autodetect/state/ModelSnapshotTests.java | 6 +-- .../autodetect/state/QuantilesTests.java | 5 ++ .../test/ml/delete_model_snapshot.yml | 4 +- .../rest-api-spec/test/ml/index_layout.yml | 46 +++++++++++++++++++ .../rest-api-spec/test/ml/post_data.yml | 2 +- .../test/ml/revert_model_snapshot.yml | 4 +- .../test/ml/update_model_snapshot.yml | 4 +- 14 files changed, 89 insertions(+), 22 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java index 6189afb06d8..a130017c3d9 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobStorageDeletionTask.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.bulk.BulkItemResponse; +import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.index.reindex.BulkByScrollResponse; import org.elasticsearch.index.reindex.DeleteByQueryAction; @@ -94,7 +95,7 @@ public class JobStorageDeletionTask extends Task { failureHandler); // Step 3. Delete quantiles done, delete the categorizer state - ActionListener deleteQuantilesHandler = ActionListener.wrap( + ActionListener deleteQuantilesHandler = ActionListener.wrap( response -> deleteCategorizerState(jobId, client, deleteCategorizerStateHandler), failureHandler); @@ -107,14 +108,19 @@ public class JobStorageDeletionTask extends Task { deleteModelState(jobId, client, deleteStateHandler); } - private void deleteQuantiles(String jobId, Client client, ActionListener finishedHandler) { - client.prepareDelete(AnomalyDetectorsIndex.jobStateIndexName(), Quantiles.TYPE.getPreferredName(), Quantiles.documentId(jobId)) - .execute(ActionListener.wrap( - finishedHandler::onResponse, + private void deleteQuantiles(String jobId, Client client, ActionListener finishedHandler) { + // The quantiles doc Id changed in v5.5 so delete both the old and new format + BulkRequestBuilder bulkRequestBuilder = client.prepareBulk(); + bulkRequestBuilder.add(client.prepareDelete(AnomalyDetectorsIndex.jobStateIndexName(), Quantiles.TYPE.getPreferredName(), + Quantiles.documentId(jobId))); + bulkRequestBuilder.add(client.prepareDelete(AnomalyDetectorsIndex.jobStateIndexName(), Quantiles.TYPE.getPreferredName(), + jobId + "-" + Quantiles.TYPE.getPreferredName())); + bulkRequestBuilder.execute(ActionListener.wrap( + response -> finishedHandler.onResponse(true), e -> { // It's not a problem for us if the index wasn't found - it's equivalent to document not found if (e instanceof IndexNotFoundException) { - finishedHandler.onResponse(new DeleteResponse()); + finishedHandler.onResponse(true); } else { finishedHandler.onFailure(e); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCounts.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCounts.java index 416568a3b67..c702fe34c46 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCounts.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCounts.java @@ -36,7 +36,7 @@ import java.util.Objects; public class DataCounts extends ToXContentToBytes implements Writeable { - private static final String DOCUMENT_SUFFIX = "-data-counts"; + private static final String DOCUMENT_SUFFIX = "_data_counts"; public static final String PROCESSED_RECORD_COUNT_STR = "processed_record_count"; public static final String PROCESSED_FIELD_COUNT_STR = "processed_field_count"; public static final String INPUT_BYTES_STR = "input_bytes"; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshot.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshot.java index 1e72307f1ad..88656a5fac4 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshot.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshot.java @@ -260,8 +260,12 @@ public class ModelSnapshot extends ToXContentToBytes implements Writeable { && this.retain == that.retain; } + private String stateDocumentPrefix() { + return jobId + "-" + snapshotId; + } + public List stateDocumentIds() { - String prefix = documentId(this); + String prefix = stateDocumentPrefix(); List stateDocumentIds = new ArrayList<>(snapshotDocCount); // The state documents count suffices are 1-based for (int i = 1; i <= snapshotDocCount; i++) { @@ -275,7 +279,7 @@ public class ModelSnapshot extends ToXContentToBytes implements Writeable { } public static String documentId(String jobId, String snapshotId) { - return jobId + "-" + snapshotId; + return jobId + "_model_snapshot_" + snapshotId; } public static ModelSnapshot fromJson(BytesReference bytesReference) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/Quantiles.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/Quantiles.java index 561ec7c94e6..552fd6031cd 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/Quantiles.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/Quantiles.java @@ -45,7 +45,7 @@ public class Quantiles extends ToXContentToBytes implements Writeable { } public static String documentId(String jobId) { - return jobId + "-" + TYPE.getPreferredName(); + return jobId + "_" + TYPE.getPreferredName(); } private final String jobId; diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/DeleteExpiredDataIT.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/DeleteExpiredDataIT.java index 65a2fe010fa..dfcb4cd6d0f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/DeleteExpiredDataIT.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/DeleteExpiredDataIT.java @@ -109,7 +109,7 @@ public class DeleteExpiredDataIT extends MlNativeAutodetectIntegTestCase { assertThat(getRecords(job.getId()).size(), equalTo(1)); List modelSnapshots = getModelSnapshots(job.getId()); assertThat(modelSnapshots.size(), equalTo(1)); - String snapshotDocId = job.getId() + "-" + modelSnapshots.get(0).getSnapshotId(); + String snapshotDocId = ModelSnapshot.documentId(modelSnapshots.get(0)); // Update snapshot timestamp to force it out of snapshot retention window String snapshotUpdate = "{ \"timestamp\": " + oneDayAgo + "}"; diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/MlDistributedFailureIT.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/MlDistributedFailureIT.java index 853bbdb002f..f0d1979b0f9 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/MlDistributedFailureIT.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/MlDistributedFailureIT.java @@ -172,7 +172,7 @@ public class MlDistributedFailureIT extends BaseMlIntegTestCase { private static DataCounts getDataCountsFromIndex(String jobId) { SearchResponse searchResponse = client().prepareSearch() .setTypes(DataCounts.TYPE.getPreferredName()) - .setQuery(QueryBuilders.idsQuery().addIds(jobId + "-data-counts")) + .setQuery(QueryBuilders.idsQuery().addIds(DataCounts.documentId(jobId))) .get(); if (searchResponse.getHits().getTotalHits() != 1) { return new DataCounts(jobId); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCountsTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCountsTests.java index c01014047b0..38249f7683f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCountsTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/DataCountsTests.java @@ -134,6 +134,12 @@ public class DataCountsTests extends AbstractSerializingTestCase { assertEquals(new Date(100L), counts.getEarliestRecordTimeStamp()); } + public void testDocumentId() { + DataCounts dataCounts = createTestInstance(); + String jobId = dataCounts.getJobid(); + assertEquals(jobId + "_data_counts", DataCounts.documentId(jobId)); + } + private void assertAllFieldsEqualZero(DataCounts stats) throws Exception { assertEquals(0L, stats.getProcessedRecordCount()); assertEquals(0L, stats.getProcessedFieldCount()); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshotTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshotTests.java index 226e6e2437b..6cb73142068 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshotTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/state/ModelSnapshotTests.java @@ -176,9 +176,9 @@ public class ModelSnapshotTests extends AbstractSerializingTestCase { assertEquals(quantiles1.hashCode(), quantiles2.hashCode()); } + public void testDocumentId() { + Quantiles quantiles = createTestInstance(); + String jobId = quantiles.getJobId(); + assertEquals(jobId + "_quantiles", Quantiles.documentId(jobId)); + } @Override protected Quantiles createTestInstance() { diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/delete_model_snapshot.yml b/plugin/src/test/resources/rest-api-spec/test/ml/delete_model_snapshot.yml index cebe711dbb2..d99248e2fe8 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/delete_model_snapshot.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/delete_model_snapshot.yml @@ -29,7 +29,7 @@ setup: index: index: .ml-anomalies-delete-model-snapshot type: model_snapshot - id: "delete-model-snapshot-inactive-snapshot" + id: "delete-model-snapshot_model_snapshot_inactive-snapshot" body: > { "job_id": "delete-model-snapshot", @@ -65,7 +65,7 @@ setup: index: index: .ml-anomalies-delete-model-snapshot type: model_snapshot - id: "delete-model-snapshot-active-snapshot" + id: "delete-model-snapshot_model_snapshot_active-snapshot" body: > { "job_id": "delete-model-snapshot", diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/index_layout.yml b/plugin/src/test/resources/rest-api-spec/test/ml/index_layout.yml index 94f8a9b276e..71067b04c2d 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/index_layout.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/index_layout.yml @@ -402,3 +402,49 @@ index: .ml-anomalies-foo - match: {count: 2} +--- +"Test delete removes 5.4 and 5.5 quantiles": + + - do: + xpack.ml.put_job: + job_id: index-layout-quantiles-job + body: > + { + "analysis_config" : { + "bucket_span": "1h", + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + "format":"xcontent" + } + } + - match: { job_id: "index-layout-quantiles-job" } + + - do: + index: + index: .ml-state + type: quantiles + id: index-layout-quantiles-job_quantiles + body: + state: quantile-state + + - do: + index: + index: .ml-state + type: quantiles + id: index-layout-quantiles-job-quantiles + body: + state: quantile-state + + - do: + xpack.ml.delete_job: + job_id: "index-layout-quantiles-job" + - match: { acknowledged: true } + + - do: + indices.refresh: {} + + - do: + count: + index: .ml-state + - match: {count: 0} diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/post_data.yml b/plugin/src/test/resources/rest-api-spec/test/ml/post_data.yml index 907bb289dff..34fbc529550 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/post_data.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/post_data.yml @@ -81,7 +81,7 @@ setup: get: index: .ml-anomalies-post-data-job type: data_counts - id: post-data-job-data-counts + id: post-data-job_data_counts - match: { _source.processed_record_count: 2 } - match: { _source.processed_field_count: 4} diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/revert_model_snapshot.yml b/plugin/src/test/resources/rest-api-spec/test/ml/revert_model_snapshot.yml index daf9563080b..f040150d58d 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/revert_model_snapshot.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/revert_model_snapshot.yml @@ -29,7 +29,7 @@ setup: index: index: .ml-anomalies-revert-model-snapshot type: model_snapshot - id: "revert-model-snapshot-first" + id: "revert-model-snapshot_model_snapshot_first" body: > { "job_id": "revert-model-snapshot", @@ -54,7 +54,7 @@ setup: index: index: .ml-anomalies-revert-model-snapshot type: model_snapshot - id: "revert-model-snapshot-second" + id: "revert-model-snapshot_model_snapshot_second" body: > { "job_id": "revert-model-snapshot", diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/update_model_snapshot.yml b/plugin/src/test/resources/rest-api-spec/test/ml/update_model_snapshot.yml index b0d5ae79f04..104c27b0c6a 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/update_model_snapshot.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/update_model_snapshot.yml @@ -17,7 +17,7 @@ setup: index: index: .ml-anomalies-update-model-snapshot type: model_snapshot - id: "update-model-snapshot-snapshot-1" + id: "update-model-snapshot_model_snapshot_snapshot-1" body: > { "job_id" : "update-model-snapshot", @@ -31,7 +31,7 @@ setup: index: index: .ml-anomalies-update-model-snapshot type: model_snapshot - id: "update-model-snapshot-snapshot-2" + id: "update-model-snapshot_model_snapshot_snapshot-2" body: > { "job_id": "update-model-snapshot",