From a4c92eb67e62027714ddfdc73245389003f00583 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Tue, 23 Dec 2014 20:10:06 -0500 Subject: [PATCH] Snapshot/Restore: add validation of restored persistent settings Closes #8830 --- .../TransportClusterUpdateSettingsAction.java | 4 +- .../cluster/settings/DynamicSettings.java | 5 ++ .../snapshots/RestoreService.java | 28 ++++++++++- .../DedicatedClusterSnapshotRestoreTests.java | 48 ++++++++++++++----- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 28a0a57436085..40a491de25320 100644 --- a/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -186,7 +186,7 @@ public ClusterState execute(final ClusterState currentState) { ImmutableSettings.Builder transientSettings = ImmutableSettings.settingsBuilder(); transientSettings.put(currentState.metaData().transientSettings()); for (Map.Entry entry : request.transientSettings().getAsMap().entrySet()) { - if (dynamicSettings.hasDynamicSetting(entry.getKey()) || entry.getKey().startsWith("logger.")) { + if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) { String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue()); if (error == null) { transientSettings.put(entry.getKey(), entry.getValue()); @@ -203,7 +203,7 @@ public ClusterState execute(final ClusterState currentState) { ImmutableSettings.Builder persistentSettings = ImmutableSettings.settingsBuilder(); persistentSettings.put(currentState.metaData().persistentSettings()); for (Map.Entry entry : request.persistentSettings().getAsMap().entrySet()) { - if (dynamicSettings.hasDynamicSetting(entry.getKey()) || entry.getKey().startsWith("logger.")) { + if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) { String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue()); if (error == null) { persistentSettings.put(entry.getKey(), entry.getValue()); diff --git a/src/main/java/org/elasticsearch/cluster/settings/DynamicSettings.java b/src/main/java/org/elasticsearch/cluster/settings/DynamicSettings.java index 25f1c1f728ea3..1848a9251f71a 100644 --- a/src/main/java/org/elasticsearch/cluster/settings/DynamicSettings.java +++ b/src/main/java/org/elasticsearch/cluster/settings/DynamicSettings.java @@ -31,6 +31,11 @@ public class DynamicSettings { private ImmutableMap dynamicSettings = ImmutableMap.of(); + + public boolean isDynamicOrLoggingSetting(String key) { + return hasDynamicSetting(key) || key.startsWith("logger."); + } + public boolean hasDynamicSetting(String key) { for (String dynamicSetting : dynamicSettings.keySet()) { if (Regex.simpleMatch(dynamicSetting, key)) { diff --git a/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 870d69c8ba4f9..7b0f32a2e107d 100644 --- a/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -34,11 +34,14 @@ import org.elasticsearch.cluster.routing.*; import org.elasticsearch.cluster.routing.allocation.AllocationService; import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; +import org.elasticsearch.cluster.settings.ClusterDynamicSettings; +import org.elasticsearch.cluster.settings.DynamicSettings; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.shard.ShardId; @@ -92,16 +95,20 @@ public class RestoreService extends AbstractComponent implements ClusterStateLis private final MetaDataCreateIndexService createIndexService; + private final DynamicSettings dynamicSettings; + private final CopyOnWriteArrayList> listeners = new CopyOnWriteArrayList<>(); @Inject - public RestoreService(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService, TransportService transportService, AllocationService allocationService, MetaDataCreateIndexService createIndexService) { + public RestoreService(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService, TransportService transportService, + AllocationService allocationService, MetaDataCreateIndexService createIndexService, @ClusterDynamicSettings DynamicSettings dynamicSettings) { super(settings); this.clusterService = clusterService; this.repositoriesService = repositoriesService; this.transportService = transportService; this.allocationService = allocationService; this.createIndexService = createIndexService; + this.dynamicSettings = dynamicSettings; transportService.registerHandler(UPDATE_RESTORE_ACTION_NAME, new UpdateRestoreStateRequestHandler()); clusterService.add(this); } @@ -283,7 +290,24 @@ private void validateExistingIndex(IndexMetaData currentIndexMetaData, IndexMeta private void restoreGlobalStateIfRequested(MetaData.Builder mdBuilder) { if (request.includeGlobalState()) { if (metaData.persistentSettings() != null) { - mdBuilder.persistentSettings(metaData.persistentSettings()); + boolean changed = false; + ImmutableSettings.Builder persistentSettings = ImmutableSettings.settingsBuilder().put(); + for (Map.Entry entry : metaData.persistentSettings().getAsMap().entrySet()) { + if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) { + String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue()); + if (error == null) { + persistentSettings.put(entry.getKey(), entry.getValue()); + changed = true; + } else { + logger.warn("ignoring persistent setting [{}], [{}]", entry.getKey(), error); + } + } else { + logger.warn("ignoring persistent setting [{}], not dynamically updateable", entry.getKey()); + } + } + if (changed) { + mdBuilder.persistentSettings(persistentSettings.build()); + } } if (metaData.templates() != null) { // TODO: Should all existing templates be deleted first? diff --git a/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java b/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java index fad835d5e24f5..11458e9104a09 100644 --- a/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java +++ b/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java @@ -51,11 +51,12 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.discovery.zen.elect.ElectMasterService; import org.elasticsearch.index.store.support.AbstractIndexStore; +import org.elasticsearch.indices.ttl.IndicesTTLService; import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.snapshots.mockstore.MockRepositoryModule; import org.elasticsearch.test.InternalTestCluster; -import org.elasticsearch.threadpool.ThreadPool; import org.junit.Ignore; import org.junit.Test; @@ -66,6 +67,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import static com.google.common.collect.Lists.newArrayList; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; @@ -82,16 +84,29 @@ public class DedicatedClusterSnapshotRestoreTests extends AbstractSnapshotTests @Test public void restorePersistentSettingsTest() throws Exception { - logger.info("--> start node"); - internalCluster().startNode(settingsBuilder().put("gateway.type", "local")); + logger.info("--> start 2 nodes"); + Settings nodeSettings = settingsBuilder() + .put("discovery.type", "zen") + .put("discovery.zen.ping_timeout", "200ms") + .put("discovery.initial_state_timeout", "500ms") + .build(); + internalCluster().startNode(nodeSettings); Client client = client(); + String secondNode = internalCluster().startNode(nodeSettings); + + int random = randomIntBetween(10, 42); - // Add dummy persistent setting logger.info("--> set test persistent setting"); - String settingValue = "test-" + randomInt(); - client.admin().cluster().prepareUpdateSettings().setPersistentSettings(ImmutableSettings.settingsBuilder().put(ThreadPool.THREADPOOL_GROUP + "dummy.value", settingValue)).execute().actionGet(); + client.admin().cluster().prepareUpdateSettings().setPersistentSettings( + ImmutableSettings.settingsBuilder() + .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, 2) + .put(IndicesTTLService.INDICES_TTL_INTERVAL, random, TimeUnit.MINUTES)) + .execute().actionGet(); + assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState() - .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo(settingValue)); + .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(random).millis())); + assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState() + .getMetaData().persistentSettings().getAsInt(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, -1), equalTo(2)); logger.info("--> create repository"); PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo") @@ -105,14 +120,25 @@ public void restorePersistentSettingsTest() throws Exception { assertThat(client.admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap").execute().actionGet().getSnapshots().get(0).state(), equalTo(SnapshotState.SUCCESS)); logger.info("--> clean the test persistent setting"); - client.admin().cluster().prepareUpdateSettings().setPersistentSettings(ImmutableSettings.settingsBuilder().put(ThreadPool.THREADPOOL_GROUP + "dummy.value", "")).execute().actionGet(); + client.admin().cluster().prepareUpdateSettings().setPersistentSettings( + ImmutableSettings.settingsBuilder() + .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, 1) + .put(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1))) + .execute().actionGet(); assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState() - .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo("")); + .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(1).millis())); + + stopNode(secondNode); + assertThat(client.admin().cluster().prepareHealth().setWaitForNodes("1").get().isTimedOut(), equalTo(false)); logger.info("--> restore snapshot"); client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setRestoreGlobalState(true).setWaitForCompletion(true).execute().actionGet(); assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState() - .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo(settingValue)); + .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(random).millis())); + + logger.info("--> ensure that zen discovery minimum master nodes wasn't restored"); + assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState() + .getMetaData().persistentSettings().getAsInt(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, -1), not(equalTo(2))); } @Test @@ -545,7 +571,7 @@ public boolean clearData(String nodeName) { } } logger.info("--> check that at least half of the shards had some reuse: [{}]", reusedShards); - assertThat(reusedShards.size(), greaterThanOrEqualTo(numberOfShards/2)); + assertThat(reusedShards.size(), greaterThanOrEqualTo(numberOfShards / 2)); } @Test