Skip to content

Commit 7c488fa

Browse files
committed
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 elastic#6268
1 parent 2546c06 commit 7c488fa

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)