Skip to content

Commit 0ff0985

Browse files
nik9000jpountz
authored andcommitted
Limit guava caches to 31.9GB
Guava's caches have overflow issues around 32GB with our default segment count of 16 and weight of 1 unit per byte. We give them 100MB of headroom so 31.9GB. This limits the sizes of both the field data and filter caches, the two large guava caches. Closes #6268
1 parent a836496 commit 0ff0985

File tree

4 files changed

+121
-3
lines changed

4 files changed

+121
-3
lines changed

src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

+8
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@
3434
*
3535
*/
3636
public class ByteSizeValue implements Serializable, Streamable {
37+
/**
38+
* Largest size possible for Guava caches to prevent overflow. Guava's
39+
* caches use integers to track weight per segment and we always 16 segments
40+
* so caches of 32GB would always overflow that integer and they'd never be
41+
* evicted by size. We set this to 31.9GB leaving 100MB of headroom to
42+
* prevent overflow.
43+
*/
44+
public static final ByteSizeValue MAX_GUAVA_CACHE_SIZE = new ByteSizeValue(32 * ByteSizeUnit.C3 - 100 * ByteSizeUnit.C2);
3745

3846
private long size;
3947

src/main/java/org/elasticsearch/indices/cache/filter/IndicesFilterCache.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,16 @@ private void buildCache() {
123123
}
124124

125125
private void computeSizeInBytes() {
126-
this.sizeInBytes = MemorySizeValue.parseBytesSizeValueOrHeapRatio(size).bytes();
126+
long sizeInBytes = MemorySizeValue.parseBytesSizeValueOrHeapRatio(size).bytes();
127+
if (sizeInBytes > ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes()) {
128+
logger.warn("reducing requested filter cache size of [{}] to the maximum allowed size of [{}]", new ByteSizeValue(sizeInBytes),
129+
ByteSizeValue.MAX_GUAVA_CACHE_SIZE);
130+
sizeInBytes = ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes();
131+
// Even though it feels wrong for size and sizeInBytes to get out of
132+
// sync we don't update size here because it might cause the cache
133+
// to be rebuilt every time new settings are applied.
134+
}
135+
this.sizeInBytes = sizeInBytes;
127136
}
128137

129138
public void addReaderKeyToClean(Object readerKey) {

src/main/java/org/elasticsearch/indices/fielddata/cache/IndicesFieldDataCache.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,14 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL
5555
public IndicesFieldDataCache(Settings settings, IndicesFieldDataCacheListener indicesFieldDataCacheListener) {
5656
super(settings);
5757
this.indicesFieldDataCacheListener = indicesFieldDataCacheListener;
58-
final String size = componentSettings.get("size", "-1");
59-
final long sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes();
58+
String size = componentSettings.get("size", "-1");
59+
long sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes();
60+
if (sizeInBytes > ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes()) {
61+
logger.warn("reducing requested field data cache size of [{}] to the maximum allowed size of [{}]", new ByteSizeValue(sizeInBytes),
62+
ByteSizeValue.MAX_GUAVA_CACHE_SIZE);
63+
sizeInBytes = ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes();
64+
size = ByteSizeValue.MAX_GUAVA_CACHE_SIZE.toString();
65+
}
6066
final TimeValue expire = componentSettings.getAsTime("expire", null);
6167
CacheBuilder<Key, RamUsage> cacheBuilder = CacheBuilder.newBuilder()
6268
.removalListener(this);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.util;
21+
22+
import com.google.common.cache.Cache;
23+
import com.google.common.cache.CacheBuilder;
24+
import com.google.common.cache.Weigher;
25+
import org.elasticsearch.common.unit.ByteSizeValue;
26+
import org.elasticsearch.test.ElasticsearchTestCase;
27+
import org.junit.Test;
28+
29+
import static org.hamcrest.Matchers.*;
30+
31+
/**
32+
* Asserts that Guava's caches can get stuck in an overflow state where they
33+
* never clear themselves based on their "weight" policy if the weight grows
34+
* beyond MAX_INT. If the noEvictionIf* methods start failing after upgrading
35+
* Guava then the problem with Guava's caches can probably be considered fixed
36+
* and {@code ByteSizeValue#MAX_GUAVA_CACHE_SIZE} can likely be removed.
37+
*/
38+
public class GuavaCacheOverflowTest extends ElasticsearchTestCase {
39+
private final int tenMeg = ByteSizeValue.parseBytesSizeValue("10MB").bytesAsInt();
40+
41+
private Cache<Integer, Object> cache;
42+
43+
@Test
44+
public void noEvictionIfWeightMaxWeightIs32GB() {
45+
checkNoEviction("32GB");
46+
}
47+
48+
@Test
49+
public void noEvictionIfWeightMaxWeightIsGreaterThan32GB() {
50+
checkNoEviction(between(33, 50) + "GB");
51+
}
52+
53+
@Test
54+
public void evictionIfWeightSlowlyGoesOverMaxWeight() {
55+
buildCache("30GB");
56+
// Add about 100GB of weight to the cache
57+
int entries = 10240;
58+
fillCache(entries);
59+
60+
// And as expected, some are purged.
61+
int missing = 0;
62+
for (int i = 0; i < 31; i++) {
63+
if (cache.getIfPresent(i + tenMeg) == null) {
64+
missing++;
65+
}
66+
}
67+
assertThat(missing, both(greaterThan(0)).and(lessThan(entries)));
68+
}
69+
70+
private void buildCache(String size) {
71+
cache = CacheBuilder.newBuilder().concurrencyLevel(16).maximumWeight(ByteSizeValue.parseBytesSizeValue(size).bytes())
72+
.weigher(new Weigher<Integer, Object>() {
73+
@Override
74+
public int weigh(Integer key, Object value) {
75+
return key;
76+
}
77+
}).build();
78+
}
79+
80+
private void fillCache(int entries) {
81+
for (int i = 0; i < entries; i++) {
82+
cache.put(i + tenMeg, i);
83+
}
84+
}
85+
86+
private void checkNoEviction(String size) {
87+
buildCache(size);
88+
// Adds ~100GB worth of weight to the cache
89+
fillCache(10240);
90+
// But nothing has been purged!
91+
for (int i = 0; i < 10000; i++) {
92+
assertNotNull(cache.getIfPresent(i + tenMeg));
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)