Ensure shared cache is page-aligned (#70876)

This commit ensures that the shared cache can only be configured with range sizes that are aligned with the page cache
size (4KB).

server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -249,6 +249,18 @@ public class Setting<T> implements ToXContentObject {
         this(new SimpleKey(key), defaultValue, parser, properties);
     }
 
+    /**
+     * Creates a new Setting instance
+     * @param key the settings key for this setting.
+     * @param fallbackSetting a setting who's value to fallback on if this setting is not defined
+     * @param parser a parser that parses the string rep into a complex datatype.
+     * @param validator a {@link Validator} for validating this setting
+     * @param properties properties for this setting like scope, filtering...
+     */
+    public Setting(String key, Setting<T> fallbackSetting, Function<String, T> parser, Validator<T> validator, Property... properties) {
+        this(new SimpleKey(key), fallbackSetting, fallbackSetting::getRaw, parser, validator, properties);
+    }
+
     /**
      * Creates a new Setting instance
      * @param key the settings key for this setting.

x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseSearchableSnapshotsIntegTestCase.java

@@ -39,6 +39,7 @@ import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.license.LicenseService.SELF_GENERATED_LICENSE_TYPE;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.elasticsearch.xpack.searchablesnapshots.cache.shared.SharedBytes.pageAligned;
 import static org.hamcrest.Matchers.equalTo;
 
 public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnapshotIntegTestCase {
@@ -86,15 +87,15 @@ public abstract class BaseSearchableSnapshotsIntegTestCase extends AbstractSnaps
         builder.put(
             FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(),
             rarely()
-                ? new ByteSizeValue(randomIntBetween(4, 1024), ByteSizeUnit.KB)
-                : new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB)
+                ? pageAligned(new ByteSizeValue(randomIntBetween(4, 1024), ByteSizeUnit.KB))
+                : pageAligned(new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB))
         );
         if (randomBoolean()) {
             builder.put(
                 FrozenCacheService.SHARED_CACHE_RANGE_SIZE_SETTING.getKey(),
                 rarely()
-                    ? new ByteSizeValue(randomIntBetween(4, 1024), ByteSizeUnit.KB)
-                    : new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB)
+                    ? pageAligned(new ByteSizeValue(randomIntBetween(4, 1024), ByteSizeUnit.KB))
+                    : pageAligned(new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB))
             );
         }
         if (randomBoolean()) {

x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/cache/blob/SearchableSnapshotsBlobStoreCacheIntegTests.java

@@ -59,6 +59,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.DATA_TIERS_CACHE_INDEX_PREFERENCE;
 import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_BLOB_CACHE_INDEX;
+import static org.elasticsearch.xpack.searchablesnapshots.cache.shared.SharedBytes.pageAligned;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 
@@ -69,7 +70,7 @@ public class SearchableSnapshotsBlobStoreCacheIntegTests extends BaseSearchableS
 
     @BeforeClass
     public static void setUpCacheSettings() {
-        blobCacheMaxLength = new ByteSizeValue(randomLongBetween(64L, 128L), ByteSizeUnit.KB);
+        blobCacheMaxLength = pageAligned(new ByteSizeValue(randomLongBetween(64L, 128L), ByteSizeUnit.KB));
 
         final Settings.Builder builder = Settings.builder();
         // Cold (full copy) cache should be unlimited to not cause evictions

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/FrozenCacheService.java

@@ -64,19 +64,33 @@ public class FrozenCacheService implements Releasable {
 
     private static final String SHARED_CACHE_SETTINGS_PREFIX = "xpack.searchable.snapshot.shared_cache.";
 
-    public static final Setting<ByteSizeValue> SHARED_CACHE_RANGE_SIZE_SETTING = Setting.byteSizeSetting(
+    public static final Setting<ByteSizeValue> SHARED_CACHE_RANGE_SIZE_SETTING = new Setting<>(
         SHARED_CACHE_SETTINGS_PREFIX + "range_size",
-        ByteSizeValue.ofMb(16),                                 // default
+        ByteSizeValue.ofMb(16).getStringRep(),
+        s -> ByteSizeValue.parseBytesSizeValue(s, SHARED_CACHE_SETTINGS_PREFIX + "range_size"),
+        getPageSizeAlignedByteSizeValueValidator(SHARED_CACHE_SETTINGS_PREFIX + "range_size"),
         Setting.Property.NodeScope
     );
 
-    // TODO: require range size to be multiple of 4kb
-    public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_REGION_SIZE_SETTING = Setting.byteSizeSetting(
+    public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_REGION_SIZE_SETTING = new Setting<>(
         SHARED_CACHE_SETTINGS_PREFIX + "region_size",
         SHARED_CACHE_RANGE_SIZE_SETTING,
+        s -> ByteSizeValue.parseBytesSizeValue(s, SHARED_CACHE_SETTINGS_PREFIX + "region_size"),
+        getPageSizeAlignedByteSizeValueValidator(SHARED_CACHE_SETTINGS_PREFIX + "region_size"),
         Setting.Property.NodeScope
     );
 
+    private static Setting.Validator<ByteSizeValue> getPageSizeAlignedByteSizeValueValidator(String settingName) {
+        return value -> {
+            if (value.getBytes() == -1) {
+                throw new SettingsException("setting [{}] must be non-negative", settingName);
+            }
+            if (value.getBytes() % SharedBytes.PAGE_SIZE != 0L) {
+                throw new SettingsException("setting [{}] must be multiple of {}", settingName, SharedBytes.PAGE_SIZE);
+            }
+        };
+    }
+
     public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_SIZE_SETTING = new Setting<>(
         SHARED_CACHE_SETTINGS_PREFIX + "size",
         ByteSizeValue.ZERO.getStringRep(),

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/SharedBytes.java

@@ -12,6 +12,7 @@ import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.Channels;
+import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.util.concurrent.AbstractRefCounted;
 import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
 import org.elasticsearch.core.internal.io.IOUtils;
@@ -194,4 +195,12 @@ public class SharedBytes extends AbstractRefCounted {
             SharedBytes.this.decRef();
         }
     }
+
+    public static ByteSizeValue pageAligned(ByteSizeValue val) {
+        final long remainder = val.getBytes() % PAGE_SIZE;
+        if (remainder != 0L) {
+            return ByteSizeValue.ofBytes(val.getBytes() + PAGE_SIZE - remainder);
+        }
+        return val;
+    }
 }

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsTestCase.java

@@ -70,6 +70,7 @@ import java.util.concurrent.TimeUnit;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomAsciiLettersOfLengthBetween;
 import static org.elasticsearch.xpack.searchablesnapshots.cache.common.TestUtils.randomPopulateAndReads;
+import static org.elasticsearch.xpack.searchablesnapshots.cache.shared.SharedBytes.pageAligned;
 
 public abstract class AbstractSearchableSnapshotsTestCase extends ESIndexInputTestCase {
 
@@ -154,13 +155,13 @@ public abstract class AbstractSearchableSnapshotsTestCase extends ESIndexInputTe
             cacheSettings.put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), randomFrozenCacheSize());
         }
         if (randomBoolean()) {
-            cacheSettings.put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), randomFrozenCacheSize());
+            cacheSettings.put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), pageAligned(randomFrozenCacheSize()));
         }
         if (randomBoolean()) {
-            cacheSettings.put(FrozenCacheService.SHARED_CACHE_RANGE_SIZE_SETTING.getKey(), randomCacheRangeSize());
+            cacheSettings.put(FrozenCacheService.SHARED_CACHE_RANGE_SIZE_SETTING.getKey(), randomFrozenCacheRangeSize());
         }
         if (randomBoolean()) {
-            cacheSettings.put(FrozenCacheService.FROZEN_CACHE_RECOVERY_RANGE_SIZE_SETTING.getKey(), randomCacheRangeSize());
+            cacheSettings.put(FrozenCacheService.FROZEN_CACHE_RECOVERY_RANGE_SIZE_SETTING.getKey(), randomFrozenCacheRangeSize());
         }
         return new FrozenCacheService(nodeEnvironment, cacheSettings.build(), threadPool);
     }
@@ -214,16 +215,23 @@ public abstract class AbstractSearchableSnapshotsTestCase extends ESIndexInputTe
      * @return a random {@link ByteSizeValue} that can be used to set {@link CacheService#SNAPSHOT_CACHE_RANGE_SIZE_SETTING}
      */
     protected static ByteSizeValue randomCacheRangeSize() {
-        return new ByteSizeValue(
-            randomLongBetween(CacheService.MIN_SNAPSHOT_CACHE_RANGE_SIZE.getBytes(), CacheService.MAX_SNAPSHOT_CACHE_RANGE_SIZE.getBytes())
+        return pageAligned(
+            new ByteSizeValue(
+                randomLongBetween(
+                    CacheService.MIN_SNAPSHOT_CACHE_RANGE_SIZE.getBytes(),
+                    CacheService.MAX_SNAPSHOT_CACHE_RANGE_SIZE.getBytes()
+                )
+            )
         );
     }
 
     protected static ByteSizeValue randomFrozenCacheRangeSize() {
-        return new ByteSizeValue(
-            randomLongBetween(
-                FrozenCacheService.MIN_SNAPSHOT_CACHE_RANGE_SIZE.getBytes(),
-                FrozenCacheService.MAX_SNAPSHOT_CACHE_RANGE_SIZE.getBytes()
+        return pageAligned(
+            new ByteSizeValue(
+                randomLongBetween(
+                    FrozenCacheService.MIN_SNAPSHOT_CACHE_RANGE_SIZE.getBytes(),
+                    FrozenCacheService.MAX_SNAPSHOT_CACHE_RANGE_SIZE.getBytes()
+                )
             )
         );
     }

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/shared/FrozenCacheServiceTests.java

@@ -10,6 +10,7 @@ package org.elasticsearch.xpack.searchablesnapshots.cache.shared;
 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.NodeEnvironment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.shard.ShardId;
@@ -28,11 +29,15 @@ import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
 
 public class FrozenCacheServiceTests extends ESTestCase {
 
+    private static long size(long numPages) {
+        return numPages * SharedBytes.PAGE_SIZE;
+    }
+
     public void testBasicEviction() throws IOException {
         Settings settings = Settings.builder()
             .put(NODE_NAME_SETTING.getKey(), "node")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), "500b")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), "100b")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(size(500)).getStringRep())
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), new ByteSizeValue(size(100)).getStringRep())
             .put("path.home", createTempDir())
             .build();
         final DeterministicTaskQueue taskQueue = new DeterministicTaskQueue(settings, random());
@@ -42,14 +47,14 @@ public class FrozenCacheServiceTests extends ESTestCase {
         ) {
             final CacheKey cacheKey = generateCacheKey();
             assertEquals(5, cacheService.freeRegionCount());
-            final CacheFileRegion region0 = cacheService.get(cacheKey, 250, 0);
-            assertEquals(100L, region0.tracker.getLength());
+            final CacheFileRegion region0 = cacheService.get(cacheKey, size(250), 0);
+            assertEquals(size(100), region0.tracker.getLength());
             assertEquals(4, cacheService.freeRegionCount());
-            final CacheFileRegion region1 = cacheService.get(cacheKey, 250, 1);
-            assertEquals(100L, region1.tracker.getLength());
+            final CacheFileRegion region1 = cacheService.get(cacheKey, size(250), 1);
+            assertEquals(size(100), region1.tracker.getLength());
             assertEquals(3, cacheService.freeRegionCount());
-            final CacheFileRegion region2 = cacheService.get(cacheKey, 250, 2);
-            assertEquals(50L, region2.tracker.getLength());
+            final CacheFileRegion region2 = cacheService.get(cacheKey, size(250), 2);
+            assertEquals(size(50), region2.tracker.getLength());
             assertEquals(2, cacheService.freeRegionCount());
 
             assertTrue(region1.tryEvict());
@@ -76,8 +81,8 @@ public class FrozenCacheServiceTests extends ESTestCase {
     public void testAutoEviction() throws IOException {
         Settings settings = Settings.builder()
             .put(NODE_NAME_SETTING.getKey(), "node")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), "200b")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), "100b")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(size(200)).getStringRep())
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), new ByteSizeValue(size(100)).getStringRep())
             .put("path.home", createTempDir())
             .build();
         final DeterministicTaskQueue taskQueue = new DeterministicTaskQueue(settings, random());
@@ -87,18 +92,18 @@ public class FrozenCacheServiceTests extends ESTestCase {
         ) {
             final CacheKey cacheKey = generateCacheKey();
             assertEquals(2, cacheService.freeRegionCount());
-            final CacheFileRegion region0 = cacheService.get(cacheKey, 250, 0);
-            assertEquals(100L, region0.tracker.getLength());
+            final CacheFileRegion region0 = cacheService.get(cacheKey, size(250), 0);
+            assertEquals(size(100), region0.tracker.getLength());
             assertEquals(1, cacheService.freeRegionCount());
-            final CacheFileRegion region1 = cacheService.get(cacheKey, 250, 1);
-            assertEquals(100L, region1.tracker.getLength());
+            final CacheFileRegion region1 = cacheService.get(cacheKey, size(250), 1);
+            assertEquals(size(100), region1.tracker.getLength());
             assertEquals(0, cacheService.freeRegionCount());
             assertFalse(region0.isEvicted());
             assertFalse(region1.isEvicted());
 
             // acquire region 2, which should evict region 0 (oldest)
-            final CacheFileRegion region2 = cacheService.get(cacheKey, 250, 2);
-            assertEquals(50L, region2.tracker.getLength());
+            final CacheFileRegion region2 = cacheService.get(cacheKey, size(250), 2);
+            assertEquals(size(50), region2.tracker.getLength());
             assertEquals(0, cacheService.freeRegionCount());
             assertTrue(region0.isEvicted());
             assertFalse(region1.isEvicted());
@@ -112,8 +117,8 @@ public class FrozenCacheServiceTests extends ESTestCase {
     public void testForceEviction() throws IOException {
         Settings settings = Settings.builder()
             .put(NODE_NAME_SETTING.getKey(), "node")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), "500b")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), "100b")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(size(500)).getStringRep())
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), new ByteSizeValue(size(100)).getStringRep())
             .put("path.home", createTempDir())
             .build();
         final DeterministicTaskQueue taskQueue = new DeterministicTaskQueue(settings, random());
@@ -124,9 +129,9 @@ public class FrozenCacheServiceTests extends ESTestCase {
             final CacheKey cacheKey1 = generateCacheKey();
             final CacheKey cacheKey2 = generateCacheKey();
             assertEquals(5, cacheService.freeRegionCount());
-            final CacheFileRegion region0 = cacheService.get(cacheKey1, 250, 0);
+            final CacheFileRegion region0 = cacheService.get(cacheKey1, size(250), 0);
             assertEquals(4, cacheService.freeRegionCount());
-            final CacheFileRegion region1 = cacheService.get(cacheKey2, 250, 1);
+            final CacheFileRegion region1 = cacheService.get(cacheKey2, size(250), 1);
             assertEquals(3, cacheService.freeRegionCount());
             assertFalse(region0.isEvicted());
             assertFalse(region1.isEvicted());
@@ -140,8 +145,8 @@ public class FrozenCacheServiceTests extends ESTestCase {
     public void testDecay() throws IOException {
         Settings settings = Settings.builder()
             .put(NODE_NAME_SETTING.getKey(), "node")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), "500b")
-            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), "100b")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(size(500)).getStringRep())
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), new ByteSizeValue(size(100)).getStringRep())
             .put("path.home", createTempDir())
             .build();
         final DeterministicTaskQueue taskQueue = new DeterministicTaskQueue(settings, random());
@@ -152,9 +157,9 @@ public class FrozenCacheServiceTests extends ESTestCase {
             final CacheKey cacheKey1 = generateCacheKey();
             final CacheKey cacheKey2 = generateCacheKey();
             assertEquals(5, cacheService.freeRegionCount());
-            final CacheFileRegion region0 = cacheService.get(cacheKey1, 250, 0);
+            final CacheFileRegion region0 = cacheService.get(cacheKey1, size(250), 0);
             assertEquals(4, cacheService.freeRegionCount());
-            final CacheFileRegion region1 = cacheService.get(cacheKey2, 250, 1);
+            final CacheFileRegion region1 = cacheService.get(cacheKey2, size(250), 1);
             assertEquals(3, cacheService.freeRegionCount());
 
             assertEquals(0, cacheService.getFreq(region0));
@@ -163,16 +168,16 @@ public class FrozenCacheServiceTests extends ESTestCase {
             taskQueue.advanceTime();
             taskQueue.runAllRunnableTasks();
 
-            final CacheFileRegion region0Again = cacheService.get(cacheKey1, 250, 0);
+            final CacheFileRegion region0Again = cacheService.get(cacheKey1, size(250), 0);
             assertSame(region0Again, region0);
             assertEquals(1, cacheService.getFreq(region0));
             assertEquals(0, cacheService.getFreq(region1));
 
             taskQueue.advanceTime();
             taskQueue.runAllRunnableTasks();
-            cacheService.get(cacheKey1, 250, 0);
+            cacheService.get(cacheKey1, size(250), 0);
             assertEquals(2, cacheService.getFreq(region0));
-            cacheService.get(cacheKey1, 250, 0);
+            cacheService.get(cacheKey1, size(250), 0);
             assertEquals(2, cacheService.getFreq(region0));
 
             // advance 2 ticks (decay only starts after 2 ticks)
@@ -196,15 +201,17 @@ public class FrozenCacheServiceTests extends ESTestCase {
             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")
+            .put(FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), new ByteSizeValue(size(500)).getStringRep())
+            .put(FrozenCacheService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), new ByteSizeValue(size(100)).getStringRep())
             .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), DataTier.DATA_HOT_NODE_ROLE.roleName())
             .build();
         FrozenCacheService.SNAPSHOT_CACHE_SIZE_SETTING.get(settings);
         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]"
+                + "] to be positive ["
+                + new ByteSizeValue(size(500)).getStringRep()
+                + "] on node without the data_frozen role is deprecated, roles are [data_hot]"
         );
     }
 

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/store/SearchableSnapshotDirectoryStatsTests.java

@@ -192,7 +192,7 @@ public class SearchableSnapshotDirectoryStatsTests extends AbstractSearchableSna
 
     public void testDirectBytesReadsWithCache() throws Exception {
         // Cache service always evicts files
-        executeTestCaseWithCache(ByteSizeValue.ZERO, randomCacheRangeSize(), (fileName, fileContent, directory) -> {
+        executeTestCaseWithCache(ByteSizeValue.ZERO, randomFrozenCacheRangeSize(), (fileName, fileContent, directory) -> {
             assertThat(directory.getStats(fileName), nullValue());
 
             final IOContext ioContext = randomIOContext();