Make searchable snapshot cache size effectively zero on non-frozen nodes by dakrone · Pull Request #70846 · elastic/elasticsearch

dakrone · 2021-03-24T22:43:13Z

This commits makes the shared_cache searchable snapshot cache size setting resolve to 0 for nodes
that to do have the data_frozen node role.

It turns out there is not a simple way to do this with the existing Settings infrastructure. Given
that setting this on non-frozen nodes is already deprecated (and already outputs a deprecated log
message), After talking to Ryan, we agreed it was better to handle this upon reading the setting
rather than adding anything to the Settings code for only this purpose.

I also attempted to make the shared cache setting private (so it actually couldn't be retrieved
outside of the static method), however, it's still needed in the SearchableSnapshots plugin
interface, so without moving it there, it has to remain public (I can move it there if we decide
that would be better).

Tangentially related to #70341

This commits makes the shared_cache searchable snapshot cache size setting resolve to 0 for nodes that to do have the `data_frozen` node role. It turns out there is not a simple way to do this with the existing Settings infrastructure. Given that setting this on non-frozen nodes is already deprecated (and already outputs a deprecated log message), After talking to Ryan, we agreed it was better to handle this upon reading the setting rather than adding anything to the `Settings` code for only this purpose. I also attempted to make the shared cache setting private (so it actually couldn't be retrieved outside of the static method), however, it's still needed in the `SearchableSnapshots` plugin interface, so without moving it there, it has to remain public (I can move it there if we decide that would be better). Tangentially related to elastic#70341

elasticmachine · 2021-03-24T22:43:16Z

Pinging @elastic/es-distributed (Team:Distributed)

…olve-to-zero

This commit converts the index metadata of searchable snapshot indices using the `shared_cache` storage type to: - Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings - Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read This is in preperation to enforcing that the `_tier_preference` setting is always set to `data_frozen` for shared cache SBIs. Relates to elastic#70846, elastic#71013, elastic#70786, elastic#70141

jasontedor · 2021-03-30T02:06:29Z

I think that we can do this in the existing settings infrastructure by overriding Setting#get(Settings). 🙈

jasontedor · 2021-03-30T02:08:22Z

Here's an untested patch that provides the concept:

diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheService.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheService.java
index b1ebbaa5cdf..ac8ca7a3e66 100644
--- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheService.java
+++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheService.java
@@ -117,7 +117,20 @@ public class FrozenCacheService implements Releasable {
 
         },
         Setting.Property.NodeScope
-    );
+    ) {
+
+        @Override
+        public ByteSizeValue get(final Settings settings) {
+            final ByteSizeValue value = super.get(settings);
+            final List<DiscoveryNodeRole> roles = NodeRoleSettings.NODE_ROLES_SETTING.get(settings);
+            if (DataTier.isFrozenNode(Set.of(roles.toArray(DiscoveryNodeRole[]::new)))) {
+                return value;
+            } else {
+                return ByteSizeValue.ZERO;
+            }
+        }
+        
+    };
 
     public static final Setting<ByteSizeValue> FROZEN_CACHE_RECOVERY_RANGE_SIZE_SETTING = Setting.byteSizeSetting(
         SHARED_CACHE_SETTINGS_PREFIX + "recovery_range_size",
diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheServiceTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheServiceTests.java
index 499b0cf4fce..d71d31cfb0f 100644
--- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheServiceTests.java
+++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/FrozenCacheServiceTests.java
@@ -10,6 +10,7 @@ package org.elasticsearch.xpack.searchablesnapshots.cache;
 import org.elasticsearch.cluster.coordination.DeterministicTaskQueue;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.shard.ShardId;
@@ -26,6 +27,7 @@ import java.nio.file.Path;
 import java.util.Set;
 
 import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
+import static org.hamcrest.Matchers.is;
 
 public class FrozenCacheServiceTests extends ESTestCase {
 
@@ -213,6 +215,24 @@ public class FrozenCacheServiceTests extends ESTestCase {
         );
     }
 
+    public void testCacheSizeClampsToZeroOnNonFrozenNodes() {
+        DiscoveryNode.setAdditionalRoles(
+            Set.of(DataTier.DATA_HOT_NODE_ROLE, DataTier.DATA_WARM_NODE_ROLE, DataTier.DATA_COLD_NODE_ROLE, DataTier.DATA_FROZEN_NODE_ROLE)
+        );
+        final Settings settings = Settings.builder()
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), "500b")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), "100b")
+            .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DataTier.DATA_HOT_NODE_ROLE.roleName())
+            .build();
+        final ByteSizeValue value = FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.get(settings);
+        assertThat(value, is(ByteSizeValue.ZERO));
+        assertWarnings(
+            "setting ["
+                + FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey()
+                + "] to be positive [500b] on node without the data_frozen role is deprecated, roles are [data_hot]"
+        );
+    }
+
     private static CacheKey generateCacheKey() {
         return new CacheKey(
             randomAlphaOfLength(10),

jasontedor

I left a suggestion.

This commit converts the index metadata of searchable snapshot indices using the `shared_cache` storage type to: - Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings - Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read This is in preperation to enforcing that the `_tier_preference` setting is always set to `data_frozen` for shared cache SBIs. Relates to #70846, #71013, #70786, #70141

This commit converts the index metadata of searchable snapshot indices using the `shared_cache` storage type to: - Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings - Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read This is in preperation to enforcing that the `_tier_preference` setting is always set to `data_frozen` for shared cache SBIs. Relates to elastic#70846, elastic#71013, elastic#70786, elastic#70141

This commits makes the shared_cache searchable snapshot cache size setting resolve to 0 for nodes that to do have the data_frozen node role. Tangentially related to elastic#70341 Supercedes elastic#70846

dakrone · 2021-03-31T17:40:27Z

@jasontedor yep that's a much better solution. I'm closing this in favor of #71134 which only targets 7.x (since you already have a PR for removing the leniency on master up)

…#71129) This commit converts the index metadata of searchable snapshot indices using the `shared_cache` storage type to: - Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings - Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read This is in preperation to enforcing that the `_tier_preference` setting is always set to `data_frozen` for shared cache SBIs. Relates to #70846, #71013, #70786, #70141

…des (#71134) * Make searchable snapshot cache size effectively zero on non-frozen nodes This commits makes the shared_cache searchable snapshot cache size setting resolve to 0 for nodes that to do have the data_frozen node role. Tangentially related to #70341 Supercedes #70846 * Update message in HasFrozenCacheAllocationDecider

dakrone added :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.13.0 labels Mar 24, 2021

elasticmachine added the Team:Distributed Meta label for distributed team. label Mar 24, 2021

dakrone added 2 commits March 25, 2021 11:49

Merge remote-tracking branch 'origin/master' into make-cache-size-res…

e3eba61

…olve-to-zero

Merge remote-tracking branch 'origin/master' into make-cache-size-res…

d3cc82b

…olve-to-zero

dakrone requested review from henningandersen, jasontedor and ywelsch March 29, 2021 20:43

dakrone mentioned this pull request Mar 29, 2021

Auto convert shared_cache SBIs to only use frozen tier #71014

Merged

jasontedor requested changes Mar 30, 2021

View reviewed changes

dakrone mentioned this pull request Mar 31, 2021

Make searchable snapshot cache size effectively zero on non-frozen nodes #71134

Merged

dakrone closed this Mar 31, 2021

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make searchable snapshot cache size effectively zero on non-frozen nodes#70846

Make searchable snapshot cache size effectively zero on non-frozen nodes#70846
dakrone wants to merge 3 commits intoelastic:masterfrom
dakrone:make-cache-size-resolve-to-zero

dakrone commented Mar 24, 2021

Uh oh!

elasticmachine commented Mar 24, 2021

Uh oh!

jasontedor commented Mar 30, 2021

Uh oh!

jasontedor commented Mar 30, 2021

Uh oh!

jasontedor left a comment

Uh oh!

dakrone commented Mar 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dakrone commented Mar 24, 2021

Uh oh!

elasticmachine commented Mar 24, 2021

Uh oh!

jasontedor commented Mar 30, 2021

Uh oh!

jasontedor commented Mar 30, 2021

Uh oh!

jasontedor left a comment

Choose a reason for hiding this comment

Uh oh!

dakrone commented Mar 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants