From dc5c31d67a714d91f05cf23d4ac30089f0ab34f2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 24 Oct 2019 09:25:50 +0200 Subject: [PATCH] Add a deprecation warning regarding allocation awareness in search request (#48351) 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 --- .../routing/EvilSystemPropertyTests.java | 1 + .../cluster/routing/OperationRouting.java | 21 +++++++++++++++++-- .../routing/OperationRoutingTests.java | 15 +++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) 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"); 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..b13377687ad67 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); + 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,7 +88,13 @@ List getAwarenessAttributes() { } private void setAwarenessAttributes(List awarenessAttributes) { - this.awarenessAttributes = awarenessAttributes; + 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; + } } public ShardIterator indexShards(ClusterState clusterState, String index, String id, @Nullable String routing) { 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); + } + }