Skip to content

Commit 4c686c1

Browse files
tlrxtlrx
authored
Add Cache Range Size setting (#51521)
This commit changes the current cache range size from 32kb to 32mb and makes it configurable through a globally defined xpack.searchable. snapshot.cache.range_size setting. This setting is set to low values in unit tests (see #50693) but a bit higher values in integration tests so that they don't take too much time to complete.
1 parent f2a4957 commit 4c686c1

File tree

6 files changed

+83
-45
lines changed

6 files changed

+83
-45
lines changed

‎x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public List<Setting<?>> getSettings() {
6161
SearchableSnapshotRepository.SNAPSHOT_SNAPSHOT_ID_SETTING,
6262
SearchableSnapshotRepository.SNAPSHOT_INDEX_ID_SETTING,
6363
SearchableSnapshotRepository.SNAPSHOT_CACHE_ENABLED_SETTING,
64-
CacheService.SNAPSHOT_CACHE_SIZE_SETTING
64+
CacheService.SNAPSHOT_CACHE_SIZE_SETTING,
65+
CacheService.SNAPSHOT_CACHE_RANGE_SIZE_SETTING
6566
);
6667
}
6768

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ public interface EvictionListener {
3434
void onEviction(CacheFile evictedCacheFile);
3535
}
3636

37-
static final int RANGE_SIZE = 1 << 15;
38-
3937
private static final StandardOpenOption[] OPEN_OPTIONS = new StandardOpenOption[]{
4038
StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.SPARSE
4139
};
@@ -51,6 +49,7 @@ protected void closeInternal() {
5149
private final ReleasableLock readLock;
5250

5351
private final SparseFileTracker tracker;
52+
private final int rangeSize;
5453
private final String name;
5554
private final Path file;
5655

@@ -60,11 +59,12 @@ protected void closeInternal() {
6059
@Nullable // if evicted, or there are no listeners
6160
private volatile FileChannel channel;
6261

63-
CacheFile(String name, long length, Path file) {
62+
CacheFile(String name, long length, Path file, int rangeSize) {
6463
this.tracker = new SparseFileTracker(file.toString(), length);
6564
this.name = Objects.requireNonNull(name);
6665
this.file = Objects.requireNonNull(file);
6766
this.listeners = new HashSet<>();
67+
this.rangeSize = rangeSize;
6868
this.evicted = false;
6969

7070
final ReentrantReadWriteLock cacheLock = new ReentrantReadWriteLock();
@@ -245,8 +245,8 @@ CompletableFuture<Integer> fetchRange(long position,
245245
}
246246

247247
ensureOpen();
248-
final long rangeStart = (position / RANGE_SIZE) * RANGE_SIZE;
249-
final long rangeEnd = Math.min(rangeStart + RANGE_SIZE, tracker.getLength());
248+
final long rangeStart = (position / rangeSize) * rangeSize;
249+
final long rangeEnd = Math.min(rangeStart + rangeSize, tracker.getLength());
250250

251251
final List<SparseFileTracker.Gap> gaps = tracker.waitForRange(rangeStart, rangeEnd,
252252
ActionListener.wrap(

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

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,42 @@
1818

1919
import java.nio.file.Files;
2020
import java.nio.file.Path;
21+
import java.util.Objects;
2122
import java.util.function.Predicate;
2223

2324
/**
2425
* {@link CacheService} maintains a cache entry for all files read from cached searchable snapshot directories (see {@link CacheDirectory})
2526
*/
2627
public class CacheService extends AbstractLifecycleComponent {
2728

28-
public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_SIZE_SETTING = Setting.byteSizeSetting("xpack.searchable.snapshot.cache.size",
29+
private static final String SETTINGS_PREFIX = "xpack.searchable.snapshot.cache.";
30+
31+
public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_SIZE_SETTING = Setting.byteSizeSetting(SETTINGS_PREFIX + "size",
2932
new ByteSizeValue(1, ByteSizeUnit.GB), // TODO: size the default value according to disk space
30-
new ByteSizeValue(0, ByteSizeUnit.BYTES), // min // NORELEASE
33+
new ByteSizeValue(0, ByteSizeUnit.BYTES), // min
34+
new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES), // max
35+
Setting.Property.NodeScope);
36+
37+
public static final Setting<ByteSizeValue> SNAPSHOT_CACHE_RANGE_SIZE_SETTING = Setting.byteSizeSetting(SETTINGS_PREFIX + "range_size",
38+
new ByteSizeValue(32, ByteSizeUnit.MB), // default
39+
new ByteSizeValue(4, ByteSizeUnit.KB), // min
3140
new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES), // max
3241
Setting.Property.NodeScope);
3342

3443
private final Cache<String, CacheFile> cache;
44+
private final ByteSizeValue cacheSize;
45+
private final ByteSizeValue rangeSize;
3546

3647
public CacheService(final Settings settings) {
48+
this(SNAPSHOT_CACHE_SIZE_SETTING.get(settings), SNAPSHOT_CACHE_RANGE_SIZE_SETTING.get(settings));
49+
}
50+
51+
// overridable by tests
52+
CacheService(final ByteSizeValue cacheSize, final ByteSizeValue rangeSize) {
53+
this.cacheSize = Objects.requireNonNull(cacheSize);
54+
this.rangeSize = Objects.requireNonNull(rangeSize);
3755
this.cache = CacheBuilder.<String, CacheFile>builder()
38-
.setMaximumWeight(SNAPSHOT_CACHE_SIZE_SETTING.get(settings).getBytes())
56+
.setMaximumWeight(cacheSize.getBytes())
3957
.weigher((key, entry) -> entry.getLength())
4058
// NORELEASE This does not immediately free space on disk, as cache file are only deleted when all index inputs
4159
// are done with reading/writing the cache file
@@ -64,6 +82,20 @@ private void ensureLifecycleStarted() {
6482
}
6583
}
6684

85+
/**
86+
* @return the cache size (in bytes)
87+
*/
88+
long getCacheSize() {
89+
return cacheSize.getBytes();
90+
}
91+
92+
/**
93+
* @return the cache range size (in bytes)
94+
*/
95+
int getRangeSize() {
96+
return Math.toIntExact(rangeSize.getBytes());
97+
}
98+
6799
public CacheFile get(final String fileName, final long length, final Path cacheDir) throws Exception {
68100
ensureLifecycleStarted();
69101
return cache.computeIfAbsent(toCacheKey(cacheDir, fileName), key -> {
@@ -74,7 +106,7 @@ public CacheFile get(final String fileName, final long length, final Path cacheD
74106
final Path path = cacheDir.resolve(uuid);
75107
assert Files.notExists(path) : "cache file already exists " + path;
76108

77-
return new CacheFile(fileName, length, path);
109+
return new CacheFile(fileName, length, path, getRangeSize());
78110
});
79111
}
80112

@@ -83,7 +115,7 @@ public CacheFile get(final String fileName, final long length, final Path cacheD
83115
*
84116
* @param predicate the predicate to evaluate
85117
*/
86-
public void removeFromCache(final Predicate<String> predicate) {
118+
void removeFromCache(final Predicate<String> predicate) {
87119
for (String cacheKey : cache.keys()) {
88120
if (predicate.test(cacheKey)) {
89121
cache.invalidate(cacheKey);

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,16 @@ protected Settings nodeSettings(int nodeOrdinal) {
5858
final Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal));
5959
if (randomBoolean()) {
6060
builder.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(),
61-
randomBoolean() ?
61+
rarely() ?
6262
new ByteSizeValue(randomIntBetween(0, 10), ByteSizeUnit.KB) :
6363
new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB));
6464
}
65+
if (randomBoolean()) {
66+
builder.put(CacheService.SNAPSHOT_CACHE_RANGE_SIZE_SETTING.getKey(),
67+
rarely() ?
68+
new ByteSizeValue(randomIntBetween(4, 1024), ByteSizeUnit.KB) :
69+
new ByteSizeValue(randomIntBetween(1, 10), ByteSizeUnit.MB));
70+
}
6571
return builder.build();
6672
}
6773

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

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.apache.lucene.store.IndexInput;
1212
import org.elasticsearch.common.lucene.store.ByteArrayIndexInput;
1313
import org.elasticsearch.common.lucene.store.ESIndexInputTestCase;
14-
import org.elasticsearch.common.settings.Settings;
1514
import org.elasticsearch.common.unit.ByteSizeUnit;
1615
import org.elasticsearch.common.unit.ByteSizeValue;
1716

@@ -21,25 +20,21 @@
2120
import java.util.Objects;
2221
import java.util.concurrent.atomic.LongAdder;
2322

24-
import static org.elasticsearch.xpack.searchablesnapshots.cache.CacheFile.RANGE_SIZE;
2523
import static org.hamcrest.Matchers.equalTo;
2624

2725
public class CacheBufferedIndexInputTests extends ESIndexInputTestCase {
2826

2927
public void testRandomReads() throws IOException {
30-
final Settings cacheSettings = randomCacheSettings();
31-
final long cacheSize = CacheService.SNAPSHOT_CACHE_SIZE_SETTING.get(cacheSettings).getBytes();
32-
33-
try (CacheService cacheService = new CacheService(cacheSettings)) {
28+
try (CacheService cacheService = createCacheService()) {
3429
cacheService.start();
3530

3631
for (int i = 0; i < 5; i++) {
3732
final String fileName = randomAlphaOfLength(10);
3833
final byte[] input = randomUnicodeOfLength(randomIntBetween(1, 100_000)).getBytes(StandardCharsets.UTF_8);
3934

4035
Directory directory = new SingleFileDirectory(fileName, input);
41-
if (input.length <= cacheSize) {
42-
directory = new CountingDirectory(directory);
36+
if (input.length <= cacheService.getCacheSize()) {
37+
directory = new CountingDirectory(directory, cacheService.getRangeSize());
4338
}
4439

4540
try (CacheDirectory cacheDirectory = new CacheDirectory(directory, cacheService, createTempDir())) {
@@ -52,7 +47,7 @@ public void testRandomReads() throws IOException {
5247
}
5348

5449
if (directory instanceof CountingDirectory) {
55-
long numberOfRanges = numberOfRanges(input.length);
50+
long numberOfRanges = numberOfRanges(input.length, cacheService.getRangeSize());
5651
assertThat("Expected " + numberOfRanges + " ranges fetched from the source",
5752
((CountingDirectory) directory).totalOpens.sum(), equalTo(numberOfRanges));
5853
assertThat("All bytes should have been read from source",
@@ -64,18 +59,17 @@ public void testRandomReads() throws IOException {
6459
}
6560
}
6661

67-
private static Settings randomCacheSettings() {
68-
final Settings.Builder cacheSettings = Settings.builder();
69-
if (randomBoolean()) {
70-
cacheSettings.put(CacheService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(),
71-
new ByteSizeValue(randomIntBetween(1, 100), randomFrom(ByteSizeUnit.BYTES, ByteSizeUnit.KB, ByteSizeUnit.MB)));
72-
}
73-
return cacheSettings.build();
62+
private CacheService createCacheService() {
63+
final ByteSizeValue cacheSize = new ByteSizeValue(randomIntBetween(1, 100),
64+
randomFrom(ByteSizeUnit.BYTES, ByteSizeUnit.KB, ByteSizeUnit.MB, ByteSizeUnit.GB));
65+
final ByteSizeValue rangeSize = new ByteSizeValue(randomIntBetween(1, 100),
66+
randomFrom(ByteSizeUnit.BYTES, ByteSizeUnit.KB, ByteSizeUnit.MB));
67+
return new CacheService(cacheSize, rangeSize);
7468
}
7569

76-
private static long numberOfRanges(int fileSize) {
77-
long numberOfRanges = fileSize / RANGE_SIZE;
78-
if (fileSize % RANGE_SIZE > 0) {
70+
private static long numberOfRanges(int fileSize, int rangeSize) {
71+
long numberOfRanges = fileSize / rangeSize;
72+
if (fileSize % rangeSize > 0) {
7973
numberOfRanges++;
8074
}
8175
if (numberOfRanges == 0) {
@@ -133,13 +127,16 @@ private static class CountingDirectory extends FilterDirectory {
133127
private final LongAdder totalBytes = new LongAdder();
134128
private final LongAdder totalOpens = new LongAdder();
135129

136-
CountingDirectory(Directory in) {
130+
private final int rangeSize;
131+
132+
CountingDirectory(Directory in, int rangeSize) {
137133
super(in);
134+
this.rangeSize = rangeSize;
138135
}
139136

140137
@Override
141138
public IndexInput openInput(String name, IOContext context) throws IOException {
142-
return new CountingIndexInput(this, super.openInput(name, context));
139+
return new CountingIndexInput(this, super.openInput(name, context), rangeSize);
143140
}
144141
}
145142

@@ -151,15 +148,17 @@ private static class CountingIndexInput extends IndexInput {
151148

152149
private final CountingDirectory dir;
153150
private final IndexInput in;
151+
private final int rangeSize;
154152

155153
private long bytesRead = 0L;
156154
private long start = Long.MAX_VALUE;
157155
private long end = Long.MIN_VALUE;
158156

159-
CountingIndexInput(CountingDirectory directory, IndexInput input) {
157+
CountingIndexInput(CountingDirectory directory, IndexInput input, int rangeSize) {
160158
super("CountingIndexInput(" + input + ")");
161159
this.dir = Objects.requireNonNull(directory);
162160
this.in = Objects.requireNonNull(input);
161+
this.rangeSize = rangeSize;
163162
dir.totalOpens.increment();
164163
}
165164

@@ -199,36 +198,36 @@ public long length() {
199198

200199
@Override
201200
public IndexInput slice(String sliceDescription, long offset, long length) throws IOException {
202-
return new CountingIndexInput(dir, in.slice(sliceDescription, offset, length));
201+
return new CountingIndexInput(dir, in.slice(sliceDescription, offset, length), rangeSize);
203202
}
204203

205204
@Override
206205
public IndexInput clone() {
207-
return new CountingIndexInput(dir, in.clone());
206+
return new CountingIndexInput(dir, in.clone(), rangeSize);
208207
}
209208

210209
@Override
211210
public void close() throws IOException {
212211
in.close();
213-
if (start % RANGE_SIZE != 0) {
212+
if (start % rangeSize != 0) {
214213
throw new AssertionError("Read operation should start at the beginning of a range");
215214
}
216-
if (end % RANGE_SIZE != 0) {
215+
if (end % rangeSize != 0) {
217216
if (end != in.length()) {
218217
throw new AssertionError("Read operation should finish at the end of a range or the end of the file");
219218
}
220219
}
221-
if (in.length() <= RANGE_SIZE) {
220+
if (in.length() <= rangeSize) {
222221
if (bytesRead != in.length()) {
223222
throw new AssertionError("All [" + in.length() + "] bytes should have been read, no more no less but got:" + bytesRead);
224223
}
225224
} else {
226-
if (bytesRead != RANGE_SIZE) {
225+
if (bytesRead != rangeSize) {
227226
if (end != in.length()) {
228-
throw new AssertionError("Expecting [" + RANGE_SIZE + "] bytes to be read but got:" + bytesRead);
227+
throw new AssertionError("Expecting [" + rangeSize + "] bytes to be read but got:" + bytesRead);
229228

230229
}
231-
final long remaining = in.length() % RANGE_SIZE;
230+
final long remaining = in.length() % rangeSize;
232231
if (bytesRead != remaining) {
233232
throw new AssertionError("Expecting [" + remaining + "] bytes to be read but got:" + bytesRead);
234233
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class CacheFileTests extends ESTestCase {
2727

2828
public void testAcquireAndRelease() throws Exception {
2929
final Path file = createTempDir().resolve("file.cache");
30-
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file);
30+
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file, randomIntBetween(1, 100));
3131

3232
assertThat("Cache file is not acquired: no channel exists", cacheFile.getChannel(), nullValue());
3333
assertThat("Cache file is not acquired: file does not exist", Files.exists(file), is(false));
@@ -70,7 +70,7 @@ public void testAcquireAndRelease() throws Exception {
7070

7171
public void testCacheFileNotAcquired() throws IOException {
7272
final Path file = createTempDir().resolve("file.cache");
73-
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file);
73+
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file, randomIntBetween(1, 100));
7474

7575
assertThat(Files.exists(file), is(false));
7676
assertThat(cacheFile.getChannel(), nullValue());
@@ -94,7 +94,7 @@ public void testCacheFileNotAcquired() throws IOException {
9494

9595
public void testDeleteOnCloseAfterLastRelease() throws Exception {
9696
final Path file = createTempDir().resolve("file.cache");
97-
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file);
97+
final CacheFile cacheFile = new CacheFile("test", randomLongBetween(1, 100), file, randomIntBetween(1, 100));
9898

9999
final List<TestEvictionListener> acquiredListeners = new ArrayList<>();
100100
for (int i = 0; i < randomIntBetween(1, 20); i++) {

0 commit comments

Comments
 (0)