From 72e982ef1bf1c441a2a1f97f22c2d3373de2c7df Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 22 Oct 2019 15:06:19 +0200 Subject: [PATCH 1/3] Add a deprecation warning regarding allocation awareness in search request This is a follow up of https://github.com/elastic/elasticsearch/issues/43453 where we added a system property to disallow allocation awareness in search requests. Since search requests will no longer check the allocation awareness attributes for routing in the next major version, this change adds a deprecation warning on any setup that uses these attributes. Relates #43453 --- .../cluster/routing/OperationRouting.java | 16 +++++++++++++++- .../cluster/routing/OperationRoutingTests.java | 15 +++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java index ea2c2b5881d9b..7057390624717 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java @@ -19,6 +19,7 @@ package org.elasticsearch.cluster.routing; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -26,6 +27,7 @@ import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; @@ -50,14 +52,23 @@ public class OperationRouting { Setting.boolSetting("cluster.routing.use_adaptive_replica_selection", true, Setting.Property.Dynamic, Setting.Property.NodeScope); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(OperationRouting.class)); + private static final String IGNORE_AWARENESS_ATTRIBUTES_PROPERTY = "es.search.ignore_awareness_attributes"; + static final String IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE = + "searches will not be routed based on awareness attributes starting in version 8.0.0; " + + "to opt into this behaviour now please set the system property [" + IGNORE_AWARENESS_ATTRIBUTES_PROPERTY + "] to [true]"; + private List awarenessAttributes; private boolean useAdaptiveReplicaSelection; public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests - boolean ignoreAwarenessAttr = parseBoolean(System.getProperty("es.search.ignore_awareness_attributes"), false); + final boolean ignoreAwarenessAttr = parseBoolean(System.getProperty(IGNORE_AWARENESS_ATTRIBUTES_PROPERTY), false); if (ignoreAwarenessAttr == false) { awarenessAttributes = AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.get(settings); + if (awarenessAttributes.isEmpty() == false) { + deprecationLogger.deprecated(IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); + } clusterSettings.addSettingsUpdateConsumer(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING, this::setAwarenessAttributes); } else { @@ -77,6 +88,9 @@ List getAwarenessAttributes() { } private void setAwarenessAttributes(List awarenessAttributes) { + if (this.awarenessAttributes.isEmpty() && awarenessAttributes.isEmpty() == false) { + deprecationLogger.deprecated(IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); + } this.awarenessAttributes = awarenessAttributes; } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/OperationRoutingTests.java index c05d0a551fe26..fe595ea8c2215 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/OperationRoutingTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.action.support.replication.ClusterStateCreationUtils; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; @@ -44,14 +45,14 @@ import java.util.Set; import java.util.TreeMap; +import static org.elasticsearch.cluster.routing.OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.object.HasToString.hasToString; -public class OperationRoutingTests extends ESTestCase{ - +public class OperationRoutingTests extends ESTestCase { public void testGenerateShardId() { int[][] possibleValues = new int[][] { {8,4,2}, {20, 10, 2}, {36, 12, 3}, {15,5,1} @@ -606,4 +607,14 @@ public void testAdaptiveReplicaSelection() throws Exception { terminate(threadPool); } + public void testAllocationAwarenessDeprecation() { + OperationRouting routing = new OperationRouting( + Settings.builder() + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "foo") + .build(), + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + assertWarnings(IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); + } + } From 937e132b7d9ebb6426285c473784880978e3f131 Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 22 Oct 2019 18:18:01 +0200 Subject: [PATCH 2/3] assert expected warning in evil system property test --- .../elasticsearch/cluster/routing/EvilSystemPropertyTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/cluster/routing/EvilSystemPropertyTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/cluster/routing/EvilSystemPropertyTests.java index 5949360a8353a..840bea04dd474 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/cluster/routing/EvilSystemPropertyTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/cluster/routing/EvilSystemPropertyTests.java @@ -34,6 +34,7 @@ public void testDisableSearchAllocationAwareness() { .build(); OperationRouting routing = new OperationRouting(indexSettings, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)); + assertWarnings(OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); assertThat(routing.getAwarenessAttributes().size(), equalTo(1)); assertThat(routing.getAwarenessAttributes().get(0), equalTo("test")); System.setProperty("es.search.ignore_awareness_attributes", "true"); From 2cb7834a6543923831e7e11c43450f39efb5f499 Mon Sep 17 00:00:00 2001 From: jimczi Date: Tue, 22 Oct 2019 18:23:48 +0200 Subject: [PATCH 3/3] address review --- .../cluster/routing/OperationRouting.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java index 7057390624717..b13377687ad67 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/OperationRouting.java @@ -63,7 +63,7 @@ public class OperationRouting { public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests - final boolean ignoreAwarenessAttr = parseBoolean(System.getProperty(IGNORE_AWARENESS_ATTRIBUTES_PROPERTY), false); + boolean ignoreAwarenessAttr = parseBoolean(System.getProperty(IGNORE_AWARENESS_ATTRIBUTES_PROPERTY), false); if (ignoreAwarenessAttr == false) { awarenessAttributes = AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.get(settings); if (awarenessAttributes.isEmpty() == false) { @@ -88,10 +88,13 @@ List getAwarenessAttributes() { } private void setAwarenessAttributes(List awarenessAttributes) { - if (this.awarenessAttributes.isEmpty() && awarenessAttributes.isEmpty() == false) { - deprecationLogger.deprecated(IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); + boolean ignoreAwarenessAttr = parseBoolean(System.getProperty(IGNORE_AWARENESS_ATTRIBUTES_PROPERTY), false); + if (ignoreAwarenessAttr == false) { + if (this.awarenessAttributes.isEmpty() && awarenessAttributes.isEmpty() == false) { + deprecationLogger.deprecated(IGNORE_AWARENESS_ATTRIBUTES_DEPRECATION_MESSAGE); + } + this.awarenessAttributes = awarenessAttributes; } - this.awarenessAttributes = awarenessAttributes; } public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) {