From aff82583981087d0ab2469c41c14c38f2c62f605 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Mon, 22 May 2017 14:44:39 +0100 Subject: [PATCH] [ML] Change result index searches to not use _type (elastic/x-pack-elasticsearch#1509) Adjusts the searches for - buckets - categories - model snapshots to not use _type. Relates elastic/x-pack-elasticsearch#668 Original commit: elastic/x-pack-elasticsearch@82696097050908db9a755f1b0727f97e123a4b61 --- .../xpack/ml/action/GetCategoriesAction.java | 22 ++++++------------- .../xpack/ml/job/persistence/JobProvider.java | 19 +++++++--------- .../rest/results/RestGetCategoriesAction.java | 12 +++++----- .../ml/action/GetCategoriesRequestTests.java | 2 +- .../AutodetectResultProcessorIT.java | 5 ++--- .../ml/job/persistence/JobProviderTests.java | 2 +- .../api/xpack.ml.get_categories.json | 2 +- .../test/ml/delete_model_snapshot.yml | 3 ++- .../test/ml/get_model_snapshots.yml | 16 ++++++++++++-- .../test/ml/jobs_get_result_categories.yml | 12 +++++----- .../test/ml/update_model_snapshot.yml | 2 ++ 11 files changed, 50 insertions(+), 47 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetCategoriesAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetCategoriesAction.java index ff527a2522f..f0a2195d968 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetCategoriesAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetCategoriesAction.java @@ -28,12 +28,12 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.ml.action.util.PageParams; +import org.elasticsearch.xpack.ml.action.util.QueryPage; import org.elasticsearch.xpack.ml.job.JobManager; import org.elasticsearch.xpack.ml.job.config.Job; import org.elasticsearch.xpack.ml.job.persistence.JobProvider; -import org.elasticsearch.xpack.ml.action.util.QueryPage; import org.elasticsearch.xpack.ml.job.results.CategoryDefinition; -import org.elasticsearch.xpack.ml.action.util.PageParams; import org.elasticsearch.xpack.ml.utils.ExceptionsHelper; import java.io.IOException; @@ -71,7 +71,7 @@ Action request.jobId = jobId, Job.ID); - PARSER.declareString(Request::setCategoryId, CATEGORY_ID); + PARSER.declareLong(Request::setCategoryId, CATEGORY_ID); PARSER.declareObject(Request::setPageParams, PageParams.PARSER, PageParams.PAGE); } @@ -84,7 +84,7 @@ Action> handler, Consumer errorHandler, Client client) { if (categoryId != null && (from != null || size != null)) { @@ -584,13 +581,10 @@ public class JobProvider { searchRequest.indicesOptions(addIgnoreUnavailable(searchRequest.indicesOptions())); SearchSourceBuilder sourceBuilder = new SearchSourceBuilder(); if (categoryId != null) { - String documentId = CategoryDefinition.documentId(jobId, categoryId); - String uid = Uid.createUid(CategoryDefinition.TYPE.getPreferredName(), documentId); - sourceBuilder.query(QueryBuilders.termQuery(UidFieldMapper.NAME, uid)); - searchRequest.routing(documentId); + sourceBuilder.query(QueryBuilders.termQuery(CategoryDefinition.CATEGORY_ID.getPreferredName(), categoryId)); } else if (from != null && size != null) { - searchRequest.types(CategoryDefinition.TYPE.getPreferredName()); sourceBuilder.from(from).size(size) + .query(QueryBuilders.existsQuery(CategoryDefinition.CATEGORY_ID.getPreferredName())) .sort(new FieldSortBuilder(CategoryDefinition.CATEGORY_ID.getPreferredName()).order(SortOrder.ASC)); } else { throw new IllegalStateException("Both categoryId and pageParams are not specified"); @@ -812,6 +806,10 @@ public class JobProvider { sortField = ModelSnapshot.TIMESTAMP.getPreferredName(); } + QueryBuilder finalQuery = QueryBuilders.boolQuery() + .filter(QueryBuilders.existsQuery(ModelSnapshot.SNAPSHOT_DOC_COUNT.getPreferredName())) + .must(qb); + FieldSortBuilder sb = new FieldSortBuilder(sortField) .order(sortDescending ? SortOrder.DESC : SortOrder.ASC); @@ -821,10 +819,9 @@ public class JobProvider { SearchRequest searchRequest = new SearchRequest(indexName); searchRequest.indicesOptions(addIgnoreUnavailable(searchRequest.indicesOptions())); - searchRequest.types(ModelSnapshot.TYPE.getPreferredName()); SearchSourceBuilder sourceBuilder = new SearchSourceBuilder(); sourceBuilder.sort(sb); - sourceBuilder.query(qb); + sourceBuilder.query(finalQuery); sourceBuilder.from(from); sourceBuilder.size(size); searchRequest.source(sourceBuilder); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java index 204453792ba..522491f695a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.ml.rest.results; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; @@ -17,8 +16,8 @@ import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.action.GetCategoriesAction; import org.elasticsearch.xpack.ml.action.GetCategoriesAction.Request; -import org.elasticsearch.xpack.ml.job.config.Job; import org.elasticsearch.xpack.ml.action.util.PageParams; +import org.elasticsearch.xpack.ml.job.config.Job; import java.io.IOException; @@ -43,23 +42,24 @@ public class RestGetCategoriesAction extends BaseRestHandler { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { Request request; String jobId = restRequest.param(Job.ID.getPreferredName()); - String categoryId = restRequest.param(Request.CATEGORY_ID.getPreferredName()); + Long categoryId = restRequest.hasParam(Request.CATEGORY_ID.getPreferredName()) ? Long.parseLong( + restRequest.param(Request.CATEGORY_ID.getPreferredName())) : null; BytesReference bodyBytes = restRequest.content(); if (bodyBytes != null && bodyBytes.length() > 0) { XContentParser parser = restRequest.contentParser(); request = GetCategoriesAction.Request.parseRequest(jobId, parser); - if (!Strings.isNullOrEmpty(categoryId)) { + if (categoryId != null) { request.setCategoryId(categoryId); } } else { request = new Request(jobId); - if (!Strings.isNullOrEmpty(categoryId)) { + if (categoryId != null) { request.setCategoryId(categoryId); } if (restRequest.hasParam(Request.FROM.getPreferredName()) || restRequest.hasParam(Request.SIZE.getPreferredName()) - || Strings.isNullOrEmpty(categoryId)){ + || categoryId == null){ request.setPageParams(new PageParams( restRequest.paramAsInt(Request.FROM.getPreferredName(), 0), diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/action/GetCategoriesRequestTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/action/GetCategoriesRequestTests.java index d842b60178d..3620bb76685 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/action/GetCategoriesRequestTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/action/GetCategoriesRequestTests.java @@ -16,7 +16,7 @@ public class GetCategoriesRequestTests extends AbstractStreamableXContentTestCas String jobId = randomAlphaOfLength(10); GetCategoriesAction.Request request = new GetCategoriesAction.Request(jobId); if (randomBoolean()) { - request.setCategoryId(randomAlphaOfLength(10)); + request.setCategoryId(randomNonNegativeLong()); } else { int from = randomInt(PageParams.MAX_FROM_SIZE_SUM); int maxSize = PageParams.MAX_FROM_SIZE_SUM - from; diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java index 342b9dad852..4f5b1bdc63f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java @@ -146,8 +146,7 @@ public class AutodetectResultProcessorIT extends XPackSingleNodeTestCase { QueryPage persistedInfluencers = getInfluencers(); assertResultsAreSame(influencers, persistedInfluencers); - QueryPage persistedDefinition = - getCategoryDefinition(Long.toString(categoryDefinition.getCategoryId())); + QueryPage persistedDefinition = getCategoryDefinition(categoryDefinition.getCategoryId()); assertEquals(1, persistedDefinition.count()); assertEquals(categoryDefinition, persistedDefinition.results().get(0)); @@ -422,7 +421,7 @@ public class AutodetectResultProcessorIT extends XPackSingleNodeTestCase { return resultHolder.get(); } - private QueryPage getCategoryDefinition(String categoryId) throws Exception { + private QueryPage getCategoryDefinition(long categoryId) throws Exception { AtomicReference errorHolder = new AtomicReference<>(); AtomicReference> resultHolder = new AtomicReference<>(); CountDownLatch latch = new CountDownLatch(1); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobProviderTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobProviderTests.java index 905fc44b3b7..321c6af9575 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobProviderTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobProviderTests.java @@ -621,7 +621,7 @@ public class JobProviderTests extends ESTestCase { String terms = "the terms and conditions are not valid here"; Map source = new HashMap<>(); - String categoryId = String.valueOf(source.hashCode()); + long categoryId = source.hashCode(); source.put("job_id", "foo"); source.put("category_id", categoryId); source.put("terms", terms); diff --git a/plugin/src/test/resources/rest-api-spec/api/xpack.ml.get_categories.json b/plugin/src/test/resources/rest-api-spec/api/xpack.ml.get_categories.json index 7e7209e3b9c..2a6a5e84b41 100644 --- a/plugin/src/test/resources/rest-api-spec/api/xpack.ml.get_categories.json +++ b/plugin/src/test/resources/rest-api-spec/api/xpack.ml.get_categories.json @@ -14,7 +14,7 @@ "description": "The name of the job" }, "category_id": { - "type" : "string", + "type" : "long", "description" : "The identifier of the category definition of interest" } }, 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 d3582dddae5..cebe711dbb2 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 @@ -73,7 +73,8 @@ setup: "snapshot_id": "active-snapshot", "description": "second", "latest_record_time_stamp": "2016-06-01T00:00:00Z", - "latest_result_time_stamp": "2016-06-01T00:00:00Z" + "latest_result_time_stamp": "2016-06-01T00:00:00Z", + "snapshot_doc_count": 3 } - do: diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/get_model_snapshots.yml b/plugin/src/test/resources/rest-api-spec/test/ml/get_model_snapshots.yml index ed9d1d5008d..d43aa709b40 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/get_model_snapshots.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/get_model_snapshots.yml @@ -18,14 +18,26 @@ setup: index: .ml-anomalies-get-model-snapshots type: model_snapshot id: "get-model-snapshots-1" - body: { "job_id": "get-model-snapshots", "snapshot_id": "1", "timestamp": "2016-06-02T00:00:00Z" } + body: > + { + "job_id": "get-model-snapshots", + "snapshot_id": "1", + "timestamp": "2016-06-02T00:00:00Z", + "snapshot_doc_count": 1 + } - do: index: index: .ml-anomalies-get-model-snapshots type: model_snapshot id: "get-model-snapshots-2" - body: { "job_id": "get-model-snapshots", "snapshot_id": "2", "timestamp": "2016-06-01T00:00:00Z" } + body: > + { + "job_id": "get-model-snapshots", + "snapshot_id": "2", + "timestamp": "2016-06-01T00:00:00Z", + "snapshot_doc_count": 2 + } - do: indices.refresh: diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yml b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yml index b757b45b9a4..90da9c994e9 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yml @@ -85,21 +85,21 @@ setup: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 from: 0 - do: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 size: 1 - do: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 from: 0 size: 1 @@ -109,7 +109,7 @@ setup: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 body: from: 0 @@ -117,7 +117,7 @@ setup: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 body: size: 1 @@ -125,7 +125,7 @@ setup: catch: request xpack.ml.get_categories: job_id: "jobs-get-result-categories" - category_id: "1" + category_id: 1 body: from: 0 size: 1 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 ab1720276cb..b0d5ae79f04 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 @@ -23,6 +23,7 @@ setup: "job_id" : "update-model-snapshot", "timestamp": "2016-06-02T00:00:00Z", "snapshot_id": "snapshot-1", + "snapshot_doc_count": 3, "retain": false } @@ -37,6 +38,7 @@ setup: "timestamp": "2016-06-01T00:00:00Z", "snapshot_id": "snapshot-2", "description": "snapshot 2 description", + "snapshot_doc_count": 2, "retain": true }