From da7aec3de4fafad4d3133e6fb2e0d17341ee5705 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 12:39:21 +0100 Subject: [PATCH 01/10] Avoid regular indices in frozen tier The frozen tier will be dedicated for partially cached searchable snapshots. This PR ensures that we do not allow allocating regular indices (including fully cached searchable snapshots) to the frozen tier. --- .../settings/AbstractScopedSettings.java | 12 ++- .../common/settings/SecureSetting.java | 6 ++ .../common/settings/ScopedSettingsTests.java | 42 +++++++++ .../routing/allocation/DataTierIT.java | 31 +++++++ .../allocation/DataTierAllocationDecider.java | 42 ++++++++- .../SearchableSnapshotsConstants.java | 15 ++++ .../DataTierAllocationDeciderTests.java | 49 +++++++++++ .../BaseSearchableSnapshotsIntegTestCase.java | 2 +- .../SearchableSnapshotDataTierIntegTests.java | 85 +++++++++++++++++++ .../SearchableSnapshotsIntegTests.java | 2 +- .../store/SearchableSnapshotDirectory.java | 2 +- .../HasFrozenCacheAllocationDecider.java | 2 +- .../SearchableSnapshotAllocator.java | 2 +- .../SearchableSnapshots.java | 11 +-- ...ransportMountSearchableSnapshotAction.java | 38 ++++++--- ...rtSearchableSnapshotCacheStoresAction.java | 2 +- ...SearchableSnapshotDirectoryStatsTests.java | 12 +-- .../store/cache/FrozenIndexInputTests.java | 3 +- .../SearchableSnapshotAllocatorTests.java | 2 +- 19 files changed, 317 insertions(+), 43 deletions(-) create mode 100644 x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java diff --git a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 3958696139170..8e14fc4831d49 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -454,7 +454,7 @@ public final void validate( * Validates that all settings are registered and valid. * * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateDependencies true if dependent settings and settings where validation has dependencies should be validated * @param ignorePrivateSettings true if private settings should be ignored during validation * @param ignoreArchivedSettings true if archived settings should be ignored during validation * @param validateInternalOrPrivateIndex true if index internal settings should be validated @@ -489,7 +489,7 @@ public final void validate( * * @param key the key of the setting to validate * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateDependencies true if dependent settings and settings where validation has dependencies should be validated * @throws IllegalArgumentException if the setting is invalid */ void validate(final String key, final Settings settings, final boolean validateDependencies) { @@ -501,7 +501,7 @@ void validate(final String key, final Settings settings, final boolean validateD * * @param key the key of the setting to validate * @param settings the settings - * @param validateDependencies true if dependent settings should be validated + * @param validateDependencies true if dependent settings and settings where validation has dependencies should be validated * @param validateInternalOrPrivateIndex true if internal index settings should be validated * @throws IllegalArgumentException if the setting is invalid */ @@ -564,7 +564,11 @@ void validate( } } } - setting.get(settings); + if (validateDependencies) { + setting.get(settings); + } else { + setting.validateWithoutDependencies(settings); + } } /** diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index 3447722322d28..bd19aacd9780b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -82,6 +82,12 @@ public T get(Settings settings) { } } + @Override + void validateWithoutDependencies(Settings settings) { + // secure settings have no Setting.Validator + get(settings); + } + /** * Returns the digest of this secure setting's value or {@code null} if the setting is missing (inside the keystore). This method can be * called even after the {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes diff --git a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index 84d8ce8855842..1ef3ff2850952 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -26,9 +26,11 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -259,6 +261,46 @@ public void testDependentSettingsWithFallback() { service.validate(Settings.builder().put("fallback.test.name", "test").put("foo.test.bar", 7).build(), true); } + public void testValidatorWithDependencies() { + final boolean nodeSetting = randomBoolean(); + final String prefix = nodeSetting ? "" : "index."; + final Property scopeProperty = nodeSetting ? Property.NodeScope : Property.IndexScope; + final Setting baseSetting = Setting.simpleString(prefix + "foo.base", Property.Dynamic, scopeProperty); + final Setting.Validator validator = new Setting.Validator() { + @Override + public void validate(String value) { + } + + @Override + public void validate(String value, Map, Object> settings, boolean isPresent) { + if (Objects.equals(value, settings.get(baseSetting)) == false) { + throw new IllegalArgumentException("must have same value"); + } + } + + @Override + public Iterator> settings() { + return List.>of(baseSetting).iterator(); + } + }; + final Setting dependingSetting = Setting.simpleString(prefix + "foo.depending", validator, scopeProperty); + + final AbstractScopedSettings service = nodeSetting ? new ClusterSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting)) : + new IndexScopedSettings(Settings.EMPTY, Set.of(baseSetting, dependingSetting)); + + service.validate(Settings.builder().put(baseSetting.getKey(), "1").put(dependingSetting.getKey(), 1).build(), true); + service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), false); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put(dependingSetting.getKey(), "1").build(), true)); + assertThat(e.getMessage(), equalTo("must have same value")); + + final IllegalArgumentException e2 = expectThrows( + IllegalArgumentException.class, + () -> service.validate(Settings.builder().put(baseSetting.getKey(), "2").put(dependingSetting.getKey(), "1").build(), true)); + assertThat(e2.getMessage(), equalTo("must have same value")); + } + public void testTupleAffixUpdateConsumer() { String prefix = randomAlphaOfLength(3) + "foo."; String intSuffix = randomAlphaOfLength(3); diff --git a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java index 28b6cd5fc6ffa..785c29722c2a9 100644 --- a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java +++ b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierIT.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.cluster.routing.allocation; +import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeType; import org.elasticsearch.action.admin.indices.template.put.PutComposableIndexTemplateAction; import org.elasticsearch.cluster.health.ClusterHealthStatus; @@ -24,6 +25,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -267,6 +269,35 @@ public void testTierFilteringIgnoredByFilterAllocationDecider() { .get(); } + public void testIllegalOnFrozen() { + startDataNode(); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> createIndex(index, Settings.builder() + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0) + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_FROZEN) + .build())); + assertThat(e.getMessage(), equalTo("[data_frozen] tier can only be used for partial searchable snapshots")); + + String initialTier = randomFrom(DataTier.DATA_HOT, DataTier.DATA_WARM, DataTier.DATA_COLD); + createIndex(index, Settings.builder() + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0) + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, initialTier) + .build()); + + IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, () -> updatePreference(DataTier.DATA_FROZEN)); + assertThat(e2.getMessage(), equalTo("[data_frozen] tier can only be used for partial searchable snapshots")); + + updatePreference(randomValueOtherThan(initialTier, () -> randomFrom(DataTier.DATA_HOT, DataTier.DATA_WARM, DataTier.DATA_COLD))); + } + + private void updatePreference(String tier) { + client().admin().indices().updateSettings(new UpdateSettingsRequest(index) + .settings(Map.of(DataTierAllocationDecider.INDEX_ROUTING_PREFER, tier))).actionGet(); + } + private DataTiersFeatureSetUsage getUsage() { XPackUsageResponse usages = new XPackUsageRequestBuilder(client()).execute().actionGet(); return usages.getUsages().stream() diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java index 845b17851725f..7d01f5522ee85 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java @@ -21,13 +21,21 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexModule; import org.elasticsearch.xpack.core.DataTier; +import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static org.elasticsearch.xpack.core.DataTier.DATA_FROZEN; + /** * The {@code DataTierAllocationDecider} is a custom allocation decider that behaves similar to the * {@link org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider}, however it @@ -45,6 +53,7 @@ public class DataTierAllocationDecider extends AllocationDecider { public static final String INDEX_ROUTING_PREFER = "index.routing.allocation.include._tier_preference"; public static final String INDEX_ROUTING_EXCLUDE = "index.routing.allocation.exclude._tier"; + private static final DataTierValidator VALIDATOR = new DataTierValidator(); public static final Setting CLUSTER_ROUTING_REQUIRE_SETTING = Setting.simpleString(CLUSTER_ROUTING_REQUIRE, DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.NodeScope); public static final Setting CLUSTER_ROUTING_INCLUDE_SETTING = Setting.simpleString(CLUSTER_ROUTING_INCLUDE, @@ -52,13 +61,13 @@ public class DataTierAllocationDecider extends AllocationDecider { public static final Setting CLUSTER_ROUTING_EXCLUDE_SETTING = Setting.simpleString(CLUSTER_ROUTING_EXCLUDE, DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.NodeScope); public static final Setting INDEX_ROUTING_REQUIRE_SETTING = Setting.simpleString(INDEX_ROUTING_REQUIRE, - DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.IndexScope); + VALIDATOR, Setting.Property.Dynamic, Setting.Property.IndexScope); public static final Setting INDEX_ROUTING_INCLUDE_SETTING = Setting.simpleString(INDEX_ROUTING_INCLUDE, - DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.IndexScope); + VALIDATOR, Setting.Property.Dynamic, Setting.Property.IndexScope); public static final Setting INDEX_ROUTING_EXCLUDE_SETTING = Setting.simpleString(INDEX_ROUTING_EXCLUDE, - DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.IndexScope); + VALIDATOR, Setting.Property.Dynamic, Setting.Property.IndexScope); public static final Setting INDEX_ROUTING_PREFER_SETTING = Setting.simpleString(INDEX_ROUTING_PREFER, - DataTierAllocationDecider::validateTierSetting, Setting.Property.Dynamic, Setting.Property.IndexScope); + VALIDATOR, Setting.Property.Dynamic, Setting.Property.IndexScope); private static void validateTierSetting(String setting) { if (Strings.hasText(setting)) { @@ -71,6 +80,31 @@ private static void validateTierSetting(String setting) { } } + private static class DataTierValidator implements Setting.Validator { + private static final Collection> dependencies = List.of(IndexModule.INDEX_STORE_TYPE_SETTING, + SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING); + + @Override + public void validate(String value) { + validateTierSetting(value); + } + + @Override + public void validate(String value, Map, Object> settings) { + if (Strings.hasText(value) && SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(settings) == false) { + String[] split = value.split(","); + if (Arrays.stream(split).anyMatch(DATA_FROZEN::equals)) { + throw new IllegalArgumentException("[" + DATA_FROZEN + "] tier can only be used for partial searchable snapshots"); + } + } + } + + @Override + public Iterator> settings() { + return dependencies.iterator(); + } + } + private volatile String clusterRequire; private volatile String clusterInclude; private volatile String clusterExclude; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java index c1b8d0e9b90af..573492bd358c3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java @@ -7,19 +7,34 @@ package org.elasticsearch.xpack.searchablesnapshots; import org.elasticsearch.Version; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import java.util.Map; + import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; public class SearchableSnapshotsConstants { public static final String SNAPSHOT_DIRECTORY_FACTORY_KEY = "snapshot"; public static final String SNAPSHOT_RECOVERY_STATE_FACTORY_KEY = "snapshot_prewarm"; + public static final Setting SNAPSHOT_PARTIAL_SETTING = Setting.boolSetting( + "index.store.snapshot.partial", + false, + Setting.Property.IndexScope, + Setting.Property.PrivateIndex, + Setting.Property.NotCopyableOnResize + ); public static boolean isSearchableSnapshotStore(Settings indexSettings) { return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(INDEX_STORE_TYPE_SETTING.get(indexSettings)); } + public static boolean isPartialSearchableSnapshotIndex(Map, Object> indexSettings) { + return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(indexSettings.get(INDEX_STORE_TYPE_SETTING)) + && (boolean) indexSettings.get(SNAPSHOT_PARTIAL_SETTING); + } + public static final String CACHE_FETCH_ASYNC_THREAD_POOL_NAME = "searchable_snapshots_cache_fetch_async"; public static final String CACHE_FETCH_ASYNC_THREAD_POOL_SETTING = "xpack.searchable_snapshots.cache_fetch_async_thread_pool"; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java index 6e4ad14457948..46b4395588511 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.cluster.routing.allocation; +import joptsimple.internal.Strings; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ESAllocationTestCase; @@ -27,14 +28,18 @@ import org.elasticsearch.cluster.routing.allocation.decider.Decision; import org.elasticsearch.cluster.routing.allocation.decider.ReplicaAfterPrimaryActiveAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.SameShardAllocationDecider; +import org.elasticsearch.common.Randomness; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexModule; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.snapshots.EmptySnapshotsInfoService; import org.elasticsearch.test.gateway.TestGatewayAllocator; import org.elasticsearch.xpack.core.DataTier; +import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -42,6 +47,7 @@ import java.util.Optional; import java.util.Set; +import static org.elasticsearch.xpack.core.DataTier.DATA_FROZEN; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -704,6 +710,49 @@ public void testExistedClusterFilters() { "tier filters [data_hot,data_warm]")); } + public void testFrozenIllegalForRegularIndices() { + List tierList = new ArrayList<>(randomSubsetOf(DataTier.ALL_DATA_TIERS)); + tierList.add(DATA_FROZEN); + Randomness.shuffle(tierList); + + String value = Strings.join(tierList, ","); + Setting setting = randomTierSetting(); + Settings.Builder builder = Settings.builder().put(setting.getKey(), value); + if (randomBoolean()) { + builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY); + } + + Settings settings = builder.build(); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> setting.get(settings)); + assertThat(exception.getMessage(), equalTo("[data_frozen] tier can only be used for partial searchable snapshots")); + } + + public void testFrozenLegalForPartialSnapshot() { + List tierList = new ArrayList<>(randomSubsetOf(DataTier.ALL_DATA_TIERS)); + tierList.add(DATA_FROZEN); + Randomness.shuffle(tierList); + + String value = Strings.join(tierList, ","); + Setting setting = randomTierSetting(); + Settings.Builder builder = Settings.builder().put(setting.getKey(), value); + builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY); + builder.put(SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING.getKey(), true); + + Settings settings = builder.build(); + + // validate do not throw + assertThat(setting.get(settings), equalTo(value)); + } + + public Setting randomTierSetting() { + //noinspection unchecked + return randomFrom( + DataTierAllocationDecider.INDEX_ROUTING_EXCLUDE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_INCLUDE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_REQUIRE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING); + } + private ClusterState prepareState(ClusterState initialState) { return prepareState(initialState, Settings.EMPTY); } diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java index bee7a394896ca..57780d971ac7b 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java @@ -143,7 +143,7 @@ protected void mountSnapshot( storage ); - final RestoreSnapshotResponse restoreResponse = client().execute(MountSearchableSnapshotAction.INSTANCE, mountRequest).get(); + final RestoreSnapshotResponse restoreResponse = client().execute(MountSearchableSnapshotAction.INSTANCE, mountRequest).actionGet(); assertThat(restoreResponse.getRestoreInfo().successfulShards(), equalTo(getNumShards(restoredIndexName).numPrimaries)); assertThat(restoreResponse.getRestoreInfo().failedShards(), equalTo(0)); } diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java new file mode 100644 index 0000000000000..53d0da85cf59e --- /dev/null +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java @@ -0,0 +1,85 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.searchablesnapshots; + +import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; +import org.elasticsearch.xpack.core.DataTier; +import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest; + +import java.util.Map; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST) +public class SearchableSnapshotDataTierIntegTests extends BaseSearchableSnapshotsIntegTestCase { + + private static final String repoName = "test-repo"; + private static final String indexName = "test-index"; + private static final String snapshotName = "test-snapshot"; + private static final String mountedIndexName = "test-index-mounted"; + private static final Settings frozenSettings = Settings.builder() + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_FROZEN) + .build(); + + public void testPartialLegalOnFrozen() throws Exception { + createRepository(repoName, "fs"); + createIndex(indexName); + createFullSnapshot(repoName, snapshotName); + Settings mountSettings = randomFrom(Settings.EMPTY, frozenSettings); + mountSnapshot( + repoName, + snapshotName, + indexName, + mountedIndexName, + mountSettings, + MountSearchableSnapshotRequest.Storage.SHARED_CACHE + ); + + updatePreference(DataTier.DATA_FROZEN); + } + + public void testFullIllegalOnFrozen() throws Exception { + createRepository(repoName, "fs"); + createIndex(indexName); + createFullSnapshot(repoName, snapshotName); + // todo: validate if we need Exception here due to remote? + expectThrows( + IllegalArgumentException.class, + () -> mountSnapshot( + repoName, + snapshotName, + indexName, + mountedIndexName, + frozenSettings, + MountSearchableSnapshotRequest.Storage.FULL_COPY + ) + ); + Settings mountSettings = randomFrom( + Settings.EMPTY, + Settings.builder() + .put( + DataTierAllocationDecider.INDEX_ROUTING_PREFER, + randomValueOtherThan(DataTier.DATA_FROZEN, () -> randomFrom(DataTier.ALL_DATA_TIERS)) + ) + .build() + ); + mountSnapshot(repoName, snapshotName, indexName, mountedIndexName, mountSettings, MountSearchableSnapshotRequest.Storage.FULL_COPY); + + expectThrows(IllegalArgumentException.class, () -> updatePreference(DataTier.DATA_FROZEN)); + } + + private void updatePreference(String tier) { + client().admin() + .indices() + .updateSettings( + new UpdateSettingsRequest(mountedIndexName).settings(Map.of(DataTierAllocationDecider.INDEX_ROUTING_PREFER, tier)) + ) + .actionGet(); + } +} diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java index ce64e7d098607..bfda4bbeecb2f 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java @@ -553,7 +553,7 @@ public void testCreateAndRestorePartialSearchableSnapshot() throws Exception { assertThat(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.get(settings).toString(), equalTo("false")); assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings), equalTo(expectedReplicas)); assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(settings), equalTo(expectedDataTiersPreference)); - assertTrue(SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING.get(settings)); + assertTrue(SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING.get(settings)); assertTrue(DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.get(settings)); checkSoftDeletesNotEagerlyLoaded(restoredIndexName); diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/SearchableSnapshotDirectory.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/SearchableSnapshotDirectory.java index 3cb7c192b5fce..31a58c0b79859 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/SearchableSnapshotDirectory.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/SearchableSnapshotDirectory.java @@ -94,7 +94,7 @@ import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_CACHE_PREWARM_ENABLED_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_ID_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_NAME_SETTING; -import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_REPOSITORY_NAME_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_REPOSITORY_UUID_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_ID_SETTING; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/HasFrozenCacheAllocationDecider.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/HasFrozenCacheAllocationDecider.java index 2a4d4df06b676..fbdf446eabe6c 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/HasFrozenCacheAllocationDecider.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/HasFrozenCacheAllocationDecider.java @@ -18,7 +18,7 @@ import org.elasticsearch.xpack.searchablesnapshots.cache.FrozenCacheInfoService; import static org.elasticsearch.snapshots.SnapshotsService.SNAPSHOT_CACHE_SIZE_SETTING; -import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING; public class HasFrozenCacheAllocationDecider extends AllocationDecider { diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotAllocator.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotAllocator.java index e269c319be73a..3e7b2aad942cd 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotAllocator.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotAllocator.java @@ -60,7 +60,7 @@ import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_ID_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_NAME_SETTING; -import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_REPOSITORY_NAME_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_ID_SETTING; import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_NAME_SETTING; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java index b4fedce5134f7..80f7c2e78266d 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java @@ -183,13 +183,6 @@ public class SearchableSnapshots extends Plugin implements IndexStorePlugin, Eng Setting.Property.NodeScope, Setting.Property.NotCopyableOnResize ); - public static final Setting SNAPSHOT_PARTIAL_SETTING = Setting.boolSetting( - "index.store.snapshot.partial", - false, - Setting.Property.IndexScope, - Setting.Property.PrivateIndex, - Setting.Property.NotCopyableOnResize - ); public static final String SNAPSHOT_BLOB_CACHE_METADATA_FILES_MAX_LENGTH = "index.store.snapshot.blob_cache.metadata_files.max_length"; public static final Setting SNAPSHOT_BLOB_CACHE_METADATA_FILES_MAX_LENGTH_SETTING = new Setting<>( new Setting.SimpleKey(SNAPSHOT_BLOB_CACHE_METADATA_FILES_MAX_LENGTH), @@ -292,7 +285,7 @@ public List> getSettings() { SNAPSHOT_CACHE_PREWARM_ENABLED_SETTING, SNAPSHOT_CACHE_EXCLUDED_FILE_TYPES_SETTING, SNAPSHOT_UNCACHED_CHUNK_SIZE_SETTING, - SNAPSHOT_PARTIAL_SETTING, + SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING, SNAPSHOT_BLOB_CACHE_METADATA_FILES_MAX_LENGTH_SETTING, CacheService.SNAPSHOT_CACHE_SIZE_SETTING, CacheService.SNAPSHOT_CACHE_RANGE_SIZE_SETTING, @@ -428,7 +421,7 @@ public Map getDirectoryFactories() { public Optional getEngineFactory(IndexSettings indexSettings) { if (SearchableSnapshotsConstants.isSearchableSnapshotStore(indexSettings.getSettings())) { final Boolean frozen = indexSettings.getSettings().getAsBoolean("index.frozen", null); - final boolean useFrozenEngine = SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING.get(indexSettings.getSettings()) + final boolean useFrozenEngine = SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING.get(indexSettings.getSettings()) && (frozen == null || frozen.equals(Boolean.TRUE)); if (useFrozenEngine) { diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java index 014a180d0f0ae..9488c63631263 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java @@ -23,6 +23,7 @@ import org.elasticsearch.cluster.routing.allocation.decider.DiskThresholdDecider; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.SystemIndices; @@ -43,7 +44,9 @@ import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; import java.util.Arrays; +import java.util.Collection; import java.util.LinkedHashSet; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -65,6 +68,13 @@ public class TransportMountSearchableSnapshotAction extends TransportMasterNodeA MountSearchableSnapshotRequest, RestoreSnapshotResponse> { + private static final Collection> DATA_TIER_ALLOCATION_SETTINGS = List.of( + DataTierAllocationDecider.INDEX_ROUTING_EXCLUDE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_INCLUDE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_REQUIRE_SETTING, + DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING + ); + private final Client client; private final RepositoriesService repositoriesService; private final XPackLicenseState licenseState; @@ -134,7 +144,7 @@ private static Settings buildIndexSettings( .put(INDEX_RECOVERY_TYPE_SETTING.getKey(), SearchableSnapshotsConstants.SNAPSHOT_RECOVERY_STATE_FACTORY_KEY); if (storage == MountSearchableSnapshotRequest.Storage.SHARED_CACHE) { - settings.put(SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING.getKey(), true) + settings.put(SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING.getKey(), true) .put(DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.getKey(), true); } @@ -211,6 +221,20 @@ protected void masterOperation( } } + Settings indexSettings = Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) // can be overridden + .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, false) // can be overridden + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getDataTiersPreference(request.storage())) + .put(request.indexSettings()) + .put(buildIndexSettings(repoData.getUuid(), request.repositoryName(), snapshotId, indexId, request.storage())) + .build(); + + // todo: restore archives bad settings, for now we verify just the data tiers, since we know their dependencies are available + // in settings + for (Setting dataTierAllocationSetting : DATA_TIER_ALLOCATION_SETTINGS) { + dataTierAllocationSetting.get(indexSettings); + } + client.admin() .cluster() .restoreSnapshot( @@ -221,17 +245,7 @@ protected void masterOperation( .renamePattern(".+") .renameReplacement(mountedIndexName) // Pass through index settings, adding the index-level settings required to use searchable snapshots - .indexSettings( - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) // can be overridden - .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, false) // can be overridden - .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getDataTiersPreference(request.storage())) - .put(request.indexSettings()) - .put( - buildIndexSettings(repoData.getUuid(), request.repositoryName(), snapshotId, indexId, request.storage()) - ) - .build() - ) + .indexSettings(indexSettings) // Pass through ignored index settings .ignoreIndexSettings(ignoreIndexSettings.toArray(new String[0])) // Don't include global state diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/cache/TransportSearchableSnapshotCacheStoresAction.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/cache/TransportSearchableSnapshotCacheStoresAction.java index 0bc7a95fb12ec..266fff8f37e29 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/cache/TransportSearchableSnapshotCacheStoresAction.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/cache/TransportSearchableSnapshotCacheStoresAction.java @@ -32,7 +32,7 @@ import java.util.List; import java.util.Optional; -import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_PARTIAL_SETTING; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING; public class TransportSearchableSnapshotCacheStoresAction extends TransportNodesAction< TransportSearchableSnapshotCacheStoresAction.Request, diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/SearchableSnapshotDirectoryStatsTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/SearchableSnapshotDirectoryStatsTests.java index 6c3a3358656c6..b8ef2a2bc5425 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/SearchableSnapshotDirectoryStatsTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/SearchableSnapshotDirectoryStatsTests.java @@ -36,7 +36,7 @@ import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.xpack.searchablesnapshots.AbstractSearchableSnapshotsTestCase; -import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots; +import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; import org.elasticsearch.xpack.searchablesnapshots.cache.CacheService; import org.elasticsearch.xpack.searchablesnapshots.cache.FrozenCacheService; @@ -543,7 +543,7 @@ private void executeTestCase(final TriConsumer Date: Tue, 9 Mar 2021 13:49:52 +0100 Subject: [PATCH 02/10] Fixes Blob cache index not on frozen. When cloning frozen have to specify the tier --- .../searchablesnapshots/SearchableSnapshotsIntegTests.java | 4 +++- .../xpack/searchablesnapshots/SearchableSnapshots.java | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java index bfda4bbeecb2f..a99e3100685d8 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java @@ -219,7 +219,8 @@ public void testCreateAndRestoreSearchableSnapshot() throws Exception { } final String expectedDataTiersPreference; if (randomBoolean()) { - expectedDataTiersPreference = String.join(",", randomSubsetOf(DataTier.ALL_DATA_TIERS)); + expectedDataTiersPreference = String.join(",", randomSubsetOf(DataTier.ALL_DATA_TIERS.stream() + .filter(tier -> tier.equals(DataTier.DATA_FROZEN) == false).collect(Collectors.toSet()))); indexSettingsBuilder.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, expectedDataTiersPreference); } else { expectedDataTiersPreference = getDataTiersPreference(MountSearchableSnapshotRequest.Storage.FULL_COPY); @@ -687,6 +688,7 @@ public void testCreateAndRestorePartialSearchableSnapshot() throws Exception { Settings.builder() .putNull(IndexModule.INDEX_STORE_TYPE_SETTING.getKey()) .putNull(IndexModule.INDEX_RECOVERY_TYPE_SETTING.getKey()) + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_HOT) .build() ) ); diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java index 80f7c2e78266d..7573031d86664 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java @@ -210,13 +210,12 @@ public class SearchableSnapshots extends Plugin implements IndexStorePlugin, Eng ); /** - * Prefer to allocate to the cold tier, then the frozen tier, then the warm tier, then the hot tier + * Prefer to allocate to the cold tier, then the warm tier, then the hot tier * This affects the system searchable snapshot cache index (not the searchable snapshot index itself) */ public static final String DATA_TIERS_CACHE_INDEX_PREFERENCE = String.join( ",", DataTier.DATA_COLD, - DataTier.DATA_FROZEN, DataTier.DATA_WARM, DataTier.DATA_HOT ); From 55d417879a5b24383e786074e58193656cdee9d7 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 13:54:53 +0100 Subject: [PATCH 03/10] spotless --- .../SearchableSnapshotsIntegTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java index a99e3100685d8..2992ea22f5c48 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsIntegTests.java @@ -219,8 +219,12 @@ public void testCreateAndRestoreSearchableSnapshot() throws Exception { } final String expectedDataTiersPreference; if (randomBoolean()) { - expectedDataTiersPreference = String.join(",", randomSubsetOf(DataTier.ALL_DATA_TIERS.stream() - .filter(tier -> tier.equals(DataTier.DATA_FROZEN) == false).collect(Collectors.toSet()))); + expectedDataTiersPreference = String.join( + ",", + randomSubsetOf( + DataTier.ALL_DATA_TIERS.stream().filter(tier -> tier.equals(DataTier.DATA_FROZEN) == false).collect(Collectors.toSet()) + ) + ); indexSettingsBuilder.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, expectedDataTiersPreference); } else { expectedDataTiersPreference = getDataTiersPreference(MountSearchableSnapshotRequest.Storage.FULL_COPY); From 98bf7272c7f5359c99fd0157895d98da4618e10c Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 15:16:32 +0100 Subject: [PATCH 04/10] Disable some tests temporarily These tests are going to be touched by other work, will wait merging this until that is done. --- .../xpack/ilm/actions/SearchableSnapshotActionIT.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index 63376295c7518..0b2b4728fa8e3 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java @@ -362,6 +362,7 @@ SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo))), } @SuppressWarnings("unchecked") + @AwaitsFix(bugUrl = "restricted frozen phase") public void testIdenticalSearchableSnapshotActionIsNoop() throws Exception { String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT); createSnapshotRepo(client(), snapshotRepo, randomBoolean()); @@ -466,6 +467,7 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception } @SuppressWarnings("unchecked") + @AwaitsFix(bugUrl = "restricted frozen phase") public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { String index = "myindex-" + randomAlphaOfLength(4).toLowerCase(Locale.ROOT) +"-000001"; createSnapshotRepo(client(), snapshotRepo, randomBoolean()); @@ -475,10 +477,11 @@ public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.SHARED_CACHE), RolloverAction.NAME, new RolloverAction(null, null, null, 1L))), - null, null, - new Phase("frozen", TimeValue.ZERO, + null, + new Phase("cold", TimeValue.ZERO, singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY))), + null, null ); @@ -504,7 +507,7 @@ RolloverAction.NAME, new RolloverAction(null, null, null, 1L))), assertBusy(() -> { Step.StepKey stepKeyForIndex = getStepKeyForIndex(client(), searchableSnapMountedIndexName); - assertThat(stepKeyForIndex.getPhase(), is("frozen")); + assertThat(stepKeyForIndex.getPhase(), is("cold")); assertThat(stepKeyForIndex.getName(), is(PhaseCompleteStep.NAME)); }, 30, TimeUnit.SECONDS); @@ -543,6 +546,7 @@ public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Except containsString("policy specifies [searchable_snapshot] action multiple times with differing repositories")); } + @AwaitsFix(bugUrl = "restricted frozen phase") public void testSearchableSnapshotActionOverridesMigrateAction() throws Exception { createSnapshotRepo(client(), snapshotRepo, randomBoolean()); createPolicy(client(), policy, From fc7503a4181101f4b5cfe4384a45fd2f71caca67 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 15:22:17 +0100 Subject: [PATCH 05/10] Revert half-baked changes. --- .../xpack/ilm/actions/SearchableSnapshotActionIT.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index 0b2b4728fa8e3..00cc990db131f 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java @@ -477,11 +477,10 @@ public void testConvertingPartialSearchableSnapshotIntoFull() throws Exception { SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.SHARED_CACHE), RolloverAction.NAME, new RolloverAction(null, null, null, 1L))), - null, - new Phase("cold", TimeValue.ZERO, + null, null, + new Phase("frozen", TimeValue.ZERO, singletonMap(SearchableSnapshotAction.NAME, new SearchableSnapshotAction(snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY))), - null, null ); @@ -507,7 +506,7 @@ RolloverAction.NAME, new RolloverAction(null, null, null, 1L))), assertBusy(() -> { Step.StepKey stepKeyForIndex = getStepKeyForIndex(client(), searchableSnapMountedIndexName); - assertThat(stepKeyForIndex.getPhase(), is("cold")); + assertThat(stepKeyForIndex.getPhase(), is("frozen")); assertThat(stepKeyForIndex.getName(), is(PhaseCompleteStep.NAME)); }, 30, TimeUnit.SECONDS); From 8734624a199efeac31d927c6d33ba41fee0d15a5 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 15:58:04 +0100 Subject: [PATCH 06/10] fix --- .../org/elasticsearch/xpack/ilm/actions/ReadonlyActionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ReadonlyActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ReadonlyActionIT.java index 2a10f07275213..1fc9155bfd161 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ReadonlyActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ReadonlyActionIT.java @@ -53,7 +53,7 @@ public void testReadOnly() throws Exception { createIndexWithSettings(client(), index, alias, Settings.builder() .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)); - String phaseName = randomFrom("warm", "cold", "frozen"); + String phaseName = randomFrom("warm", "cold"); createNewSingletonPolicy(client(), policy, phaseName, new ReadOnlyAction()); updatePolicy(client(), index, policy); assertBusy(() -> { From 2831e2852097a0e623dc0dff93b82f04ad696cc5 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Tue, 9 Mar 2021 17:35:53 +0100 Subject: [PATCH 07/10] smaller fixes --- .../allocation/DataTierAllocationDeciderTests.java | 8 ++++++-- .../SearchableSnapshotDataTierIntegTests.java | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java index 46b4395588511..a45a74c08ed20 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderTests.java @@ -712,7 +712,9 @@ public void testExistedClusterFilters() { public void testFrozenIllegalForRegularIndices() { List tierList = new ArrayList<>(randomSubsetOf(DataTier.ALL_DATA_TIERS)); - tierList.add(DATA_FROZEN); + if (tierList.contains(DATA_FROZEN) == false) { + tierList.add(DATA_FROZEN); + } Randomness.shuffle(tierList); String value = Strings.join(tierList, ","); @@ -729,7 +731,9 @@ public void testFrozenIllegalForRegularIndices() { public void testFrozenLegalForPartialSnapshot() { List tierList = new ArrayList<>(randomSubsetOf(DataTier.ALL_DATA_TIERS)); - tierList.add(DATA_FROZEN); + if (tierList.contains(DATA_FROZEN) == false) { + tierList.add(DATA_FROZEN); + } Randomness.shuffle(tierList); String value = Strings.join(tierList, ","); diff --git a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java index 53d0da85cf59e..6f3686032c984 100644 --- a/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java +++ b/x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotDataTierIntegTests.java @@ -48,7 +48,6 @@ public void testFullIllegalOnFrozen() throws Exception { createRepository(repoName, "fs"); createIndex(indexName); createFullSnapshot(repoName, snapshotName); - // todo: validate if we need Exception here due to remote? expectThrows( IllegalArgumentException.class, () -> mountSnapshot( From af0212660b96c2904d441bbd9f673c1134951afb Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Wed, 10 Mar 2021 10:52:29 +0100 Subject: [PATCH 08/10] Disable bwc Old 7.12/7.13 clusters will create the .snapshot-blob-cache index with tier preference including frozen, which causes cluster state updates to fail when upgrading. Disabling bwc until this is in 7.12. --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 47652b111c926..0c4c4c7075681 100644 --- a/build.gradle +++ b/build.gradle @@ -189,8 +189,8 @@ tasks.register("verifyVersions") { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = true -String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ +boolean bwc_tests_enabled = false +String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/70141" /* place a PR link here when committing bwc changes */ /* * FIPS 140-2 behavior was fixed in 7.11.0. Before that there is no way to run elasticsearch in a * JVM that is properly configured to be in fips mode with BCFIPS. For now we need to disable From 20087bdce93eb22d1173ba85ce7931a7d0098772 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Wed, 10 Mar 2021 10:52:29 +0100 Subject: [PATCH 09/10] Add assertions --- .../SearchableSnapshotsConstants.java | 7 ++++ .../SearchableSnapshotsConstantsTests.java | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstantsTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java index 573492bd358c3..5ad93d8955360 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java @@ -30,7 +30,14 @@ public static boolean isSearchableSnapshotStore(Settings indexSettings) { return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(INDEX_STORE_TYPE_SETTING.get(indexSettings)); } + /** + * Based on a map from setting to value, do the settings represent a partial searchable snapshot index? + * + * Both index.store.type and index.store.snapshot.partial must be supplied. + */ public static boolean isPartialSearchableSnapshotIndex(Map, Object> indexSettings) { + assert indexSettings.containsKey(INDEX_STORE_TYPE_SETTING) : "must include store type in map"; + assert indexSettings.get(SNAPSHOT_PARTIAL_SETTING) != null : "partial setting must be non-null in map (has default value)"; return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(indexSettings.get(INDEX_STORE_TYPE_SETTING)) && (boolean) indexSettings.get(SNAPSHOT_PARTIAL_SETTING); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstantsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstantsTests.java new file mode 100644 index 0000000000000..25c81bb9caa73 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstantsTests.java @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.searchablesnapshots; + +import org.elasticsearch.index.IndexModule; +import org.elasticsearch.test.ESTestCase; + +import java.util.Map; + +import static org.hamcrest.Matchers.is; + +public class SearchableSnapshotsConstantsTests extends ESTestCase { + + public void testIsPartialSearchableSnapshotIndex() { + assertThat(SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex( + Map.of(IndexModule.INDEX_STORE_TYPE_SETTING, SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY, + SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING, false)), + is(false)); + + assertThat(SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex( + Map.of(IndexModule.INDEX_STORE_TYPE_SETTING, "abc", + SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING, randomBoolean())), + is(false)); + + assertThat(SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex( + Map.of(IndexModule.INDEX_STORE_TYPE_SETTING, SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY, + SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING, true)), + is(true)); + } +} From 771d61637a9b8aa9ec1297610a8cee84bc2655ed Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 11 Mar 2021 07:35:20 +0100 Subject: [PATCH 10/10] Remove SecureSetting change. --- .../org/elasticsearch/common/settings/SecureSetting.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index bd19aacd9780b..3447722322d28 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -82,12 +82,6 @@ public T get(Settings settings) { } } - @Override - void validateWithoutDependencies(Settings settings) { - // secure settings have no Setting.Validator - get(settings); - } - /** * Returns the digest of this secure setting's value or {@code null} if the setting is missing (inside the keystore). This method can be * called even after the {@code SecureSettings} have been closed, unlike {@code #get(Settings)}. The digest is used to check for changes