From d8b29f7beb37993c77bfb3298a77513651ef81d4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 21 Sep 2015 22:09:26 -0400 Subject: [PATCH] Fix ping timeout settings inconsistencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes ping timeout settings inconsistencies in ZenDiscovery. In particular, the documentation refers to the ping timeout setting as discovery.zen.ping_timeout but the code was ultimately using discovery.zen.ping.timeout if this was set. This commit also changes all instances of the raw string “discovery.zen.ping_timeout” to the constant o.e.d.z.ZenDiscovery.SETTING_PING_TIMEOUT. Finally, this commit removes the legacy setting "discovery.zen.initial_ping_timeout". Closes #6579, #9581, #9908 --- .../discovery/zen/ZenDiscovery.java | 9 +++------ .../elasticsearch/cluster/ClusterServiceIT.java | 16 ++++------------ .../cluster/MinimumMasterNodesIT.java | 8 ++++---- .../elasticsearch/cluster/NoMasterNodeIT.java | 13 +++++-------- .../DedicatedClusterSnapshotRestoreIT.java | 17 ++++------------- docs/reference/migration/migrate_3_0.asciidoc | 11 ++++++++++- .../azure/AzureMinimumMasterNodesTests.java | 5 +++-- 7 files changed, 33 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java index 204f9e98f94..be8dcb59516 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java @@ -76,7 +76,7 @@ import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds; public class ZenDiscovery extends AbstractLifecycleComponent implements Discovery, PingContextProvider { public final static String SETTING_REJOIN_ON_MASTER_GONE = "discovery.zen.rejoin_on_master_gone"; - public final static String SETTING_PING_TIMEOUT = "discovery.zen.ping.timeout"; + public final static String SETTING_PING_TIMEOUT = "discovery.zen.ping_timeout"; public final static String SETTING_JOIN_TIMEOUT = "discovery.zen.join_timeout"; public final static String SETTING_JOIN_RETRY_ATTEMPTS = "discovery.zen.join_retry_attempts"; public final static String SETTING_JOIN_RETRY_DELAY = "discovery.zen.join_retry_delay"; @@ -150,10 +150,7 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen this.discoverySettings = discoverySettings; this.pingService = pingService; this.electMaster = electMasterService; - TimeValue pingTimeout = this.settings.getAsTime("discovery.zen.initial_ping_timeout", timeValueSeconds(3)); - pingTimeout = this.settings.getAsTime("discovery.zen.ping_timeout", pingTimeout); - pingTimeout = settings.getAsTime("discovery.zen.ping_timeout", pingTimeout); - this.pingTimeout = settings.getAsTime(SETTING_PING_TIMEOUT, pingTimeout); + this.pingTimeout = settings.getAsTime(SETTING_PING_TIMEOUT, timeValueSeconds(3)); this.joinTimeout = settings.getAsTime(SETTING_JOIN_TIMEOUT, TimeValue.timeValueMillis(this.pingTimeout.millis() * 20)); this.joinRetryAttempts = settings.getAsInt(SETTING_JOIN_RETRY_ATTEMPTS, 3); @@ -173,7 +170,7 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen throw new IllegalArgumentException("'" + SETTING_MAX_PINGS_FROM_ANOTHER_MASTER + "' must be a positive number. got [" + this.maxPingsFromAnotherMaster + "]"); } - logger.debug("using ping.timeout [{}], join.timeout [{}], master_election.filter_client [{}], master_election.filter_data [{}]", pingTimeout, joinTimeout, masterElectionFilterClientNodes, masterElectionFilterDataNodes); + logger.debug("using ping_timeout [{}], join.timeout [{}], master_election.filter_client [{}], master_election.filter_data [{}]", this.pingTimeout, joinTimeout, masterElectionFilterClientNodes, masterElectionFilterDataNodes); nodeSettingsService.addListener(new ApplySettings()); diff --git a/core/src/test/java/org/elasticsearch/cluster/ClusterServiceIT.java b/core/src/test/java/org/elasticsearch/cluster/ClusterServiceIT.java index 72327e41b7c..a0760dbe1aa 100644 --- a/core/src/test/java/org/elasticsearch/cluster/ClusterServiceIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/ClusterServiceIT.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.inject.Singleton; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.discovery.zen.ZenDiscovery; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; @@ -42,12 +43,7 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.ThreadPool; import org.junit.Test; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import java.util.*; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -55,11 +51,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.test.ESIntegTestCase.Scope; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.*; /** * @@ -634,7 +626,7 @@ public class ClusterServiceIT extends ESIntegTestCase { Settings settings = settingsBuilder() .put("discovery.type", "zen") .put("discovery.zen.minimum_master_nodes", 1) - .put("discovery.zen.ping_timeout", "400ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "400ms") .put("discovery.initial_state_timeout", "500ms") .build(); diff --git a/core/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java b/core/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java index ab01ea16ea7..41712b97c0d 100644 --- a/core/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java @@ -69,7 +69,7 @@ public class MinimumMasterNodesIT extends ESIntegTestCase { Settings settings = settingsBuilder() .put("discovery.type", "zen") .put("discovery.zen.minimum_master_nodes", 2) - .put("discovery.zen.ping_timeout", "200ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "200ms") .put("discovery.initial_state_timeout", "500ms") .build(); @@ -182,7 +182,7 @@ public class MinimumMasterNodesIT extends ESIntegTestCase { Settings settings = settingsBuilder() .put("discovery.type", "zen") .put("discovery.zen.minimum_master_nodes", 3) - .put("discovery.zen.ping_timeout", "1s") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "1s") .put("discovery.initial_state_timeout", "500ms") .build(); @@ -258,7 +258,7 @@ public class MinimumMasterNodesIT extends ESIntegTestCase { public void dynamicUpdateMinimumMasterNodes() throws Exception { Settings settings = settingsBuilder() .put("discovery.type", "zen") - .put("discovery.zen.ping_timeout", "400ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "400ms") .put("discovery.initial_state_timeout", "500ms") .build(); @@ -317,7 +317,7 @@ public class MinimumMasterNodesIT extends ESIntegTestCase { int nodeCount = scaledRandomIntBetween(1, 5); Settings.Builder settings = settingsBuilder() .put("discovery.type", "zen") - .put("discovery.zen.ping_timeout", "200ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "200ms") .put("discovery.initial_state_timeout", "500ms"); // set an initial value which is at least quorum to avoid split brains during initial startup diff --git a/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java b/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java index 2d3630c2e66..fd8d6d6d155 100644 --- a/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/NoMasterNodeIT.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.discovery.DiscoverySettings; import org.elasticsearch.discovery.MasterNotDiscoveredException; +import org.elasticsearch.discovery.zen.ZenDiscovery; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; @@ -45,12 +46,8 @@ import java.util.HashMap; import static org.elasticsearch.action.percolate.PercolateSourceBuilder.docBuilder; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertExists; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*; +import static org.hamcrest.Matchers.*; /** */ @@ -68,7 +65,7 @@ public class NoMasterNodeIT extends ESIntegTestCase { .put("discovery.type", "zen") .put("action.auto_create_index", autoCreateIndex) .put("discovery.zen.minimum_master_nodes", 2) - .put("discovery.zen.ping_timeout", "200ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "200ms") .put("discovery.initial_state_timeout", "500ms") .put(DiscoverySettings.NO_MASTER_BLOCK, "all") .build(); @@ -221,7 +218,7 @@ public class NoMasterNodeIT extends ESIntegTestCase { .put("discovery.type", "zen") .put("action.auto_create_index", false) .put("discovery.zen.minimum_master_nodes", 2) - .put("discovery.zen.ping_timeout", "200ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "200ms") .put("discovery.initial_state_timeout", "500ms") .put(DiscoverySettings.NO_MASTER_BLOCK, "write") .build(); diff --git a/core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index 6249232d70d..d886ce719b1 100644 --- a/core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -50,6 +50,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.discovery.zen.ZenDiscovery; import org.elasticsearch.discovery.zen.elect.ElectMasterService; import org.elasticsearch.index.store.IndexStore; import org.elasticsearch.indices.recovery.RecoveryState; @@ -81,18 +82,8 @@ import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.test.ESIntegTestCase.ClusterScope; import static org.elasticsearch.test.ESIntegTestCase.Scope; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows; -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*; +import static org.hamcrest.Matchers.*; /** */ @@ -110,7 +101,7 @@ public class DedicatedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTest logger.info("--> start 2 nodes"); Settings nodeSettings = settingsBuilder() .put("discovery.type", "zen") - .put("discovery.zen.ping_timeout", "200ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "200ms") .put("discovery.initial_state_timeout", "500ms") .build(); internalCluster().startNode(nodeSettings); diff --git a/docs/reference/migration/migrate_3_0.asciidoc b/docs/reference/migration/migrate_3_0.asciidoc index 1d61fcaea19..5654d8d04ff 100644 --- a/docs/reference/migration/migrate_3_0.asciidoc +++ b/docs/reference/migration/migrate_3_0.asciidoc @@ -87,4 +87,13 @@ previous versions. When `max_children` was set to `0` on the `has_child` query then there was no upper limit on how many children documents are allowed to match. This has changed and `0` now really means to zero child documents are allowed. If no upper limit -is needed then the `max_children` option shouldn't be defined at all on the `has_child` query. \ No newline at end of file +is needed then the `max_children` option shouldn't be defined at all on the `has_child` query. + +=== Settings changes === + +==== Ping timeout settings + +Previously, there were three settings for the ping timeout: `discovery.zen.initial_ping_timeout`, +`discovery.zen.ping.timeout` and `discovery.zen.ping_timeout`. The former two have been removed and +the only setting key for the ping timeout is now `discovery.zen.ping_timeout`. The default value for +ping timeouts remains at three seconds. diff --git a/plugins/discovery-azure/src/test/java/org/elasticsearch/discovery/azure/AzureMinimumMasterNodesTests.java b/plugins/discovery-azure/src/test/java/org/elasticsearch/discovery/azure/AzureMinimumMasterNodesTests.java index 846513623f0..75ae011e750 100644 --- a/plugins/discovery-azure/src/test/java/org/elasticsearch/discovery/azure/AzureMinimumMasterNodesTests.java +++ b/plugins/discovery-azure/src/test/java/org/elasticsearch/discovery/azure/AzureMinimumMasterNodesTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.cloud.azure.AbstractAzureComputeServiceTestCase; import org.elasticsearch.cloud.azure.AzureComputeServiceTwoNodesMock; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.discovery.MasterNotDiscoveredException; +import org.elasticsearch.discovery.zen.ZenDiscovery; import org.elasticsearch.test.ESIntegTestCase; import org.junit.Test; import org.apache.lucene.util.LuceneTestCase.AwaitsFix; @@ -53,8 +54,8 @@ public class AzureMinimumMasterNodesTests extends AbstractAzureComputeServiceTes .put(super.nodeSettings(nodeOrdinal)) .put("discovery.zen.minimum_master_nodes", 2) // Make the test run faster - .put("discovery.zen.join.timeout", "50ms") - .put("discovery.zen.ping.timeout", "10ms") + .put(ZenDiscovery.SETTING_JOIN_TIMEOUT, "50ms") + .put(ZenDiscovery.SETTING_PING_TIMEOUT, "10ms") .put("discovery.initial_state_timeout", "100ms"); return builder.build(); }