From 9506f605046c8aa05b2fd57cd430f207a4334a78 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 20 Jun 2016 13:10:35 +0200 Subject: [PATCH] Improve error message if a setting is not found (#18920) Today we only emit that the setting wasn't found unless we have some DYM suggestions. Yet, if a setting is not found at all and there are no suggestions due to typos it's likely a removed setting or the plugin that is supposed to be configured is not installed. This commit adds some info text to the exception to help the user debugging the problem before opening bugreports. Instead of emitting: `unknown setting [foo.bar]` we now emit: `unknown setting [foo.bar] please check the migration guide for removed settings and ensure that the plugin you are configuring is installed` Relates to #18663 --- .../resources/checkstyle_suppressions.xml | 2 -- .../settings/AbstractScopedSettings.java | 3 +++ .../admin/indices/create/CreateIndexIT.java | 26 ++++++++++++------- .../common/settings/ScopedSettingsTests.java | 6 +++-- .../common/settings/SettingsModuleTests.java | 6 +++-- .../indices/state/SimpleIndexStateIT.java | 12 ++++++--- .../template/SimpleIndexTemplateIT.java | 3 ++- 7 files changed, 38 insertions(+), 20 deletions(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index ffad8d37c5b..27b9efbf0a4 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -712,7 +712,6 @@ - @@ -1022,7 +1021,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 0170c4218a2..817e109bf4d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -263,6 +263,9 @@ public abstract class AbstractScopedSettings extends AbstractComponent { List keys = scoredKeys.stream().map((a) -> a.v2()).collect(Collectors.toList()); if (keys.isEmpty() == false) { msg += " did you mean " + (keys.size() == 1 ? "[" + keys.get(0) + "]": "any of " + keys.toString()) + "?"; + } else { + msg += " please check that any required plugins are installed, or check the breaking changes documentation for removed " + + "settings"; } throw new IllegalArgumentException(msg); } diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index 8fbb489d9c2..3e7323dceeb 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -70,7 +70,8 @@ public class CreateIndexIT extends ESIntegTestCase { prepareCreate("test").setSettings(Settings.builder().put(IndexMetaData.SETTING_CREATION_DATE, 4L)).get(); fail(); } catch (IllegalArgumentException ex) { - assertEquals("unknown setting [index.creation_date]", ex.getMessage()); + assertEquals("unknown setting [index.creation_date] please check that any required plugins are installed, or check the " + + "breaking changes documentation for removed settings", ex.getMessage()); } } @@ -165,7 +166,8 @@ public class CreateIndexIT extends ESIntegTestCase { .get(); fail("should have thrown an exception about the shard count"); } catch (IllegalArgumentException e) { - assertEquals("unknown setting [index.unknown.value]", e.getMessage()); + assertEquals("unknown setting [index.unknown.value] please check that any required plugins are installed, or check the" + + " breaking changes documentation for removed settings", e.getMessage()); } } @@ -211,13 +213,16 @@ public class CreateIndexIT extends ESIntegTestCase { @Override public void run() { try { - client().prepareIndex("test", "test").setSource("index_version", indexVersion.get()).get(); // recreate that index + // recreate that index + client().prepareIndex("test", "test").setSource("index_version", indexVersion.get()).get(); synchronized (indexVersionLock) { - // we sync here since we have to ensure that all indexing operations below for a given ID are done before we increment the - // index version otherwise a doc that is in-flight could make it into an index that it was supposed to be deleted for and our assertion fail... + // we sync here since we have to ensure that all indexing operations below for a given ID are done before + // we increment the index version otherwise a doc that is in-flight could make it into an index that it + // was supposed to be deleted for and our assertion fail... indexVersion.incrementAndGet(); } - assertAcked(client().admin().indices().prepareDelete("test").get()); // from here on all docs with index_version == 0|1 must be gone!!!! only 2 are ok; + // from here on all docs with index_version == 0|1 must be gone!!!! only 2 are ok; + assertAcked(client().admin().indices().prepareDelete("test").get()); } finally { latch.countDown(); } @@ -249,8 +254,10 @@ public class CreateIndexIT extends ESIntegTestCase { latch.await(); refresh(); - // we only really assert that we never reuse segments of old indices or anything like this here and that nothing fails with crazy exceptions - SearchResponse expected = client().prepareSearch("test").setIndicesOptions(IndicesOptions.lenientExpandOpen()).setQuery(new RangeQueryBuilder("index_version").from(indexVersion.get(), true)).get(); + // we only really assert that we never reuse segments of old indices or anything like this here and that nothing fails with + // crazy exceptions + SearchResponse expected = client().prepareSearch("test").setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .setQuery(new RangeQueryBuilder("index_version").from(indexVersion.get(), true)).get(); SearchResponse all = client().prepareSearch("test").setIndicesOptions(IndicesOptions.lenientExpandOpen()).get(); assertEquals(expected + " vs. " + all, expected.getHits().getTotalHits(), all.getHits().getTotalHits()); logger.info("total: {}", expected.getHits().getTotalHits()); @@ -283,7 +290,8 @@ public class CreateIndexIT extends ESIntegTestCase { } public void testRestartIndexCreationAfterFullClusterRestart() throws Exception { - client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("cluster.routing.allocation.enable", "none")).get(); + client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("cluster.routing.allocation.enable", + "none")).get(); client().admin().indices().prepareCreate("test").setSettings(indexSettings()).get(); internalCluster().fullRestart(); ensureGreen("test"); diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index 3afd60d86e4..664f8cb96ab 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -202,20 +202,22 @@ public class ScopedSettingsTests extends ESTestCase { IndexScopedSettings settings = new IndexScopedSettings( Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); + String unknownMsgSuffix = " please check that any required plugins are installed, or check the breaking changes documentation for" + + " removed settings"; settings.validate(Settings.builder().put("index.store.type", "boom")); settings.validate(Settings.builder().put("index.store.type", "boom").build()); try { settings.validate(Settings.builder().put("index.store.type", "boom", "i.am.not.a.setting", true)); fail(); } catch (IllegalArgumentException e) { - assertEquals("unknown setting [i.am.not.a.setting]", e.getMessage()); + assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); } try { settings.validate(Settings.builder().put("index.store.type", "boom", "i.am.not.a.setting", true).build()); fail(); } catch (IllegalArgumentException e) { - assertEquals("unknown setting [i.am.not.a.setting]", e.getMessage()); + assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); } try { diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java index 353b7b61d6c..692134916ef 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsModuleTests.java @@ -51,7 +51,8 @@ public class SettingsModuleTests extends ModuleTestCase { () -> new SettingsModule(settings)); assertEquals("Failed to parse value [[2.0]] for setting [cluster.routing.allocation.balance.shard]", ex.getMessage()); assertEquals(1, ex.getSuppressed().length); - assertEquals("unknown setting [some.foo.bar]", ex.getSuppressed()[0].getMessage()); + assertEquals("unknown setting [some.foo.bar] please check that any required plugins are installed, or check the breaking " + + "changes documentation for removed settings", ex.getSuppressed()[0].getMessage()); } { @@ -127,7 +128,8 @@ public class SettingsModuleTests extends ModuleTestCase { new SettingsModule(settings); fail(); } catch (IllegalArgumentException ex) { - assertEquals("tribe.blocks validation failed: unknown setting [wtf]", ex.getMessage()); + assertEquals("tribe.blocks validation failed: unknown setting [wtf] please check that any required plugins are" + + " installed, or check the breaking changes documentation for removed settings", ex.getMessage()); } } } diff --git a/core/src/test/java/org/elasticsearch/indices/state/SimpleIndexStateIT.java b/core/src/test/java/org/elasticsearch/indices/state/SimpleIndexStateIT.java index f3898bc9f3c..ea3ebf5179b 100644 --- a/core/src/test/java/org/elasticsearch/indices/state/SimpleIndexStateIT.java +++ b/core/src/test/java/org/elasticsearch/indices/state/SimpleIndexStateIT.java @@ -56,7 +56,8 @@ public class SimpleIndexStateIT extends ESIntegTestCase { ClusterStateResponse stateResponse = client().admin().cluster().prepareState().get(); assertThat(stateResponse.getState().metaData().index("test").getState(), equalTo(IndexMetaData.State.OPEN)); assertThat(stateResponse.getState().routingTable().index("test").shards().size(), equalTo(numShards.numPrimaries)); - assertThat(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size(), equalTo(numShards.totalNumShards)); + assertEquals(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size() + , numShards.totalNumShards); logger.info("--> indexing a simple document"); client().prepareIndex("test", "type1", "1").setSource("field1", "value1").get(); @@ -88,7 +89,8 @@ public class SimpleIndexStateIT extends ESIntegTestCase { assertThat(stateResponse.getState().metaData().index("test").getState(), equalTo(IndexMetaData.State.OPEN)); assertThat(stateResponse.getState().routingTable().index("test").shards().size(), equalTo(numShards.numPrimaries)); - assertThat(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size(), equalTo(numShards.totalNumShards)); + assertEquals(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size(), + numShards.totalNumShards); logger.info("--> indexing a simple document"); client().prepareIndex("test", "type1", "1").setSource("field1", "value1").get(); @@ -119,7 +121,8 @@ public class SimpleIndexStateIT extends ESIntegTestCase { ClusterStateResponse stateResponse = client().admin().cluster().prepareState().get(); assertThat(stateResponse.getState().metaData().index("test").getState(), equalTo(IndexMetaData.State.OPEN)); assertThat(stateResponse.getState().routingTable().index("test").shards().size(), equalTo(numShards.numPrimaries)); - assertThat(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size(), equalTo(numShards.totalNumShards)); + assertEquals(stateResponse.getState().routingTable().index("test").shardsWithState(ShardRoutingState.STARTED).size(), + numShards.totalNumShards); logger.info("--> indexing a simple document"); client().prepareIndex("test", "type1", "1").setSource("field1", "value1").get(); @@ -143,7 +146,8 @@ public class SimpleIndexStateIT extends ESIntegTestCase { } logger.info("--> creating test index with valid settings "); - CreateIndexResponse response = client().admin().indices().prepareCreate("test").setSettings(Settings.builder().put("number_of_shards", 1)).get(); + CreateIndexResponse response = client().admin().indices().prepareCreate("test") + .setSettings(Settings.builder().put("number_of_shards", 1)).get(); assertThat(response.isAcknowledged(), equalTo(true)); } } diff --git a/core/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java b/core/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java index 298cb9cd9e3..493f8b74e04 100644 --- a/core/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java +++ b/core/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java @@ -322,7 +322,8 @@ public class SimpleIndexTemplateIT extends ESIntegTestCase { .setTemplate("te*") .setSettings(Settings.builder().put("does_not_exist", "test")) .get()); - assertEquals("unknown setting [index.does_not_exist]", e.getMessage()); + assertEquals("unknown setting [index.does_not_exist] please check that any required plugins are" + + " installed, or check the breaking changes documentation for removed settings", e.getMessage()); response = client().admin().indices().prepareGetTemplates().get(); assertEquals(0, response.getIndexTemplates().size());