From 45bef79a65bd91958dd13a43855ba32c4fc58a70 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Fri, 17 Aug 2018 14:46:50 -0400 Subject: [PATCH 1/2] [Rollup] Better error message when trying to set non-rollup index We don't allow the user to configure a rollup index against an existing index, but the exceptions that we return are not clear about that. They indicate issues with metadata, instead of stating the real reason (not allowed to use a non-rollup index to store rollup data). This makes the exception better, and adds a bit more testing --- .../action/TransportPutRollupJobAction.java | 8 ++-- .../action/PutJobStateMachineTests.java | 41 ++++++++++++++++++- .../rest-api-spec/test/rollup/put_job.yml | 32 +++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java index 9f20fba8e92da..98e19a2e4b0e7 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java @@ -158,7 +158,8 @@ static void updateMapping(RollupJob job, ActionListener li MappingMetaData mappings = getMappingResponse.getMappings().get(indexName).get(RollupField.TYPE_NAME); Object m = mappings.getSourceAsMap().get("_meta"); if (m == null) { - String msg = "Expected to find _meta key in mapping of rollup index [" + indexName + "] but not found."; + String msg = "Rollup indices cannot co-mingle with non-rollup data (expected to find _meta key in " + + "mapping of rollup index [" + indexName + "] but not found)."; logger.error(msg); listener.onFailure(new RuntimeException(msg)); return; @@ -166,8 +167,9 @@ static void updateMapping(RollupJob job, ActionListener li Map metadata = (Map) m; if (metadata.get(RollupField.ROLLUP_META) == null) { - String msg = "Expected to find rollup meta key [" + RollupField.ROLLUP_META + "] in mapping of rollup index [" + indexName - + "] but not found."; + String msg = "Rollup indices cannot co-mingle with non-rollup data (expected to find " + + "rollup meta key [" + RollupField.ROLLUP_META + "] in mapping of rollup index [" + + indexName + "] but not found)."; logger.error(msg); listener.onFailure(new RuntimeException(msg)); return; diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java index 5599c50321cf1..58e25bede8a5c 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java @@ -180,8 +180,8 @@ public void testNoMetadataInMapping() { ActionListener testListener = ActionListener.wrap(response -> { fail("Listener success should not have been triggered."); }, e -> { - assertThat(e.getMessage(), equalTo("Expected to find _meta key in mapping of rollup index [" - + job.getConfig().getRollupIndex() + "] but not found.")); + assertThat(e.getMessage(), equalTo("Rollup indices cannot co-mingle with non-rollup data (expected to " + + "find _meta key in mapping of rollup index [" + job.getConfig().getRollupIndex() + "] but not found).")); }); Logger logger = mock(Logger.class); @@ -206,6 +206,43 @@ public void testNoMetadataInMapping() { verify(client).execute(eq(GetMappingsAction.INSTANCE), any(GetMappingsRequest.class), any()); } + @SuppressWarnings("unchecked") + public void testMetadataButNotRollup() { + RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap()); + + ActionListener testListener = ActionListener.wrap(response -> { + fail("Listener success should not have been triggered."); + }, e -> { + assertThat(e.getMessage(), equalTo("Rollup indices cannot co-mingle with non-rollup data (expected to " + + "find rollup meta key [_rollup] in mapping of rollup index [" + job.getConfig().getRollupIndex() + "] but not found).")); + }); + + Logger logger = mock(Logger.class); + Client client = mock(Client.class); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ActionListener.class); + doAnswer(invocation -> { + GetMappingsResponse response = mock(GetMappingsResponse.class); + Map m = new HashMap<>(2); + m.put("random", + Collections.singletonMap(job.getConfig().getId(), job.getConfig())); + MappingMetaData meta = new MappingMetaData(RollupField.TYPE_NAME, + Collections.singletonMap("_meta", m)); + ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(1); + builder.put(RollupField.TYPE_NAME, meta); + + ImmutableOpenMap.Builder> builder2 = ImmutableOpenMap.builder(1); + builder2.put(job.getConfig().getRollupIndex(), builder.build()); + + when(response.getMappings()).thenReturn(builder2.build()); + requestCaptor.getValue().onResponse(response); + return null; + }).when(client).execute(eq(GetMappingsAction.INSTANCE), any(GetMappingsRequest.class), requestCaptor.capture()); + + TransportPutRollupJobAction.updateMapping(job, testListener, mock(PersistentTasksService.class), client, logger); + verify(client).execute(eq(GetMappingsAction.INSTANCE), any(GetMappingsRequest.class), any()); + } + @SuppressWarnings("unchecked") public void testNoMappingVersion() { RollupJob job = new RollupJob(ConfigTestHelpers.randomRollupJobConfig(random()), Collections.emptyMap()); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml index 516be25be2a2d..23df0c5837700 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml @@ -128,6 +128,38 @@ setup: ] } +--- +"Test put_job in non-rollup index": + - do: + indices.create: + index: non-rollup + - do: + catch: /foo/ + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + xpack.rollup.put_job: + id: foo + body: > + { + "index_pattern": "foo", + "rollup_index": "non-rollup", + "cron": "*/30 * * * * ?", + "page_size" :10, + "groups" : { + "date_histogram": { + "field": "the_field", + "interval": "1h" + } + }, + "metrics": [ + { + "field": "value_field", + "metrics": ["min", "max", "sum"] + } + ] + } + + --- "Try to include headers": From 2ec9fd79f07edb0155b9fc7dda0aba7926fe0d0f Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 27 Aug 2018 15:06:44 -0400 Subject: [PATCH 2/2] Better message --- .../rollup/action/TransportPutRollupJobAction.java | 8 ++++---- .../xpack/rollup/action/PutJobStateMachineTests.java | 10 ++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java index 98e19a2e4b0e7..f0600d80f82a6 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java @@ -158,8 +158,8 @@ static void updateMapping(RollupJob job, ActionListener li MappingMetaData mappings = getMappingResponse.getMappings().get(indexName).get(RollupField.TYPE_NAME); Object m = mappings.getSourceAsMap().get("_meta"); if (m == null) { - String msg = "Rollup indices cannot co-mingle with non-rollup data (expected to find _meta key in " + - "mapping of rollup index [" + indexName + "] but not found)."; + String msg = "Rollup data cannot be added to existing indices that contain non-rollup data (expected " + + "to find _meta key in mapping of rollup index [" + indexName + "] but not found)."; logger.error(msg); listener.onFailure(new RuntimeException(msg)); return; @@ -167,8 +167,8 @@ static void updateMapping(RollupJob job, ActionListener li Map metadata = (Map) m; if (metadata.get(RollupField.ROLLUP_META) == null) { - String msg = "Rollup indices cannot co-mingle with non-rollup data (expected to find " + - "rollup meta key [" + RollupField.ROLLUP_META + "] in mapping of rollup index [" + String msg = "Rollup data cannot be added to existing indices that contain non-rollup data (expected " + + "to find rollup meta key [" + RollupField.ROLLUP_META + "] in mapping of rollup index [" + indexName + "] but not found)."; logger.error(msg); listener.onFailure(new RuntimeException(msg)); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java index 58e25bede8a5c..3d346456ea98d 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java @@ -180,8 +180,9 @@ public void testNoMetadataInMapping() { ActionListener testListener = ActionListener.wrap(response -> { fail("Listener success should not have been triggered."); }, e -> { - assertThat(e.getMessage(), equalTo("Rollup indices cannot co-mingle with non-rollup data (expected to " + - "find _meta key in mapping of rollup index [" + job.getConfig().getRollupIndex() + "] but not found).")); + assertThat(e.getMessage(), equalTo("Rollup data cannot be added to existing indices that contain " + + "non-rollup data (expected to find _meta key in mapping of rollup index [" + + job.getConfig().getRollupIndex() + "] but not found).")); }); Logger logger = mock(Logger.class); @@ -213,8 +214,9 @@ public void testMetadataButNotRollup() { ActionListener testListener = ActionListener.wrap(response -> { fail("Listener success should not have been triggered."); }, e -> { - assertThat(e.getMessage(), equalTo("Rollup indices cannot co-mingle with non-rollup data (expected to " + - "find rollup meta key [_rollup] in mapping of rollup index [" + job.getConfig().getRollupIndex() + "] but not found).")); + assertThat(e.getMessage(), equalTo("Rollup data cannot be added to existing indices that contain " + + "non-rollup data (expected to find rollup meta key [_rollup] in mapping of rollup index [" + + job.getConfig().getRollupIndex() + "] but not found).")); }); Logger logger = mock(Logger.class);