Skip to content

Limit guava caches to 31.9GB #6286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
*
*/
public class ByteSizeValue implements Serializable, Streamable {
/**
* Largest size possible for Guava caches to prevent overflow. Guava's
* caches use integers to track weight per segment and we always 16 segments
* so caches of 32GB would always overflow that integer and they'd never be
* evicted by size. We set this to 31.9GB leaving 100MB of headroom to
* prevent overflow.
*/
public static final ByteSizeValue MAX_GUAVA_CACHE_SIZE = new ByteSizeValue(32 * ByteSizeUnit.C3 - 100 * ByteSizeUnit.C2);

private long size;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,16 @@ private void buildCache() {
}

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

public void addReaderKeyToClean(Object readerKey) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,14 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL
public IndicesFieldDataCache(Settings settings, IndicesFieldDataCacheListener indicesFieldDataCacheListener) {
super(settings);
this.indicesFieldDataCacheListener = indicesFieldDataCacheListener;
final String size = componentSettings.get("size", "-1");
final long sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes();
String size = componentSettings.get("size", "-1");
long sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes();
if (sizeInBytes > ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes()) {
logger.warn("reducing requested field data cache size of [{}] to the maximum allowed size of [{}]", new ByteSizeValue(sizeInBytes),
ByteSizeValue.MAX_GUAVA_CACHE_SIZE);
sizeInBytes = ByteSizeValue.MAX_GUAVA_CACHE_SIZE.bytes();
size = ByteSizeValue.MAX_GUAVA_CACHE_SIZE.toString();
}
final TimeValue expire = componentSettings.getAsTime("expire", null);
CacheBuilder<Key, RamUsage> cacheBuilder = CacheBuilder.newBuilder()
.removalListener(this);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common.util;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.Weigher;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Test;

import static org.hamcrest.Matchers.*;

/**
* Asserts that Guava's caches can get stuck in an overflow state where they
* never clear themselves based on their "weight" policy if the weight grows
* beyond MAX_INT. If the noEvictionIf* methods start failing after upgrading
* Guava then the problem with Guava's caches can probably be considered fixed
* and {@code ByteSizeValue#MAX_GUAVA_CACHE_SIZE} can likely be removed.
*/
public class GuavaCacheOverflowTest extends ElasticsearchTestCase {
private final int tenMeg = ByteSizeValue.parseBytesSizeValue("10MB").bytesAsInt();

private Cache<Integer, Object> cache;

@Test
public void noEvictionIfWeightMaxWeightIs32GB() {
checkNoEviction("32GB");
}

@Test
public void noEvictionIfWeightMaxWeightIsGreaterThan32GB() {
checkNoEviction(between(33, 50) + "GB");
}

@Test
public void evictionIfWeightSlowlyGoesOverMaxWeight() {
buildCache("30GB");
// Add about 100GB of weight to the cache
int entries = 10240;
fillCache(entries);

// And as expected, some are purged.
int missing = 0;
for (int i = 0; i < 31; i++) {
if (cache.getIfPresent(i + tenMeg) == null) {
missing++;
}
}
assertThat(missing, both(greaterThan(0)).and(lessThan(entries)));
}

private void buildCache(String size) {
cache = CacheBuilder.newBuilder().concurrencyLevel(16).maximumWeight(ByteSizeValue.parseBytesSizeValue(size).bytes())
.weigher(new Weigher<Integer, Object>() {
@Override
public int weigh(Integer key, Object value) {
return key;
}
}).build();
}

private void fillCache(int entries) {
for (int i = 0; i < entries; i++) {
cache.put(i + tenMeg, i);
}
}

private void checkNoEviction(String size) {
buildCache(size);
// Adds ~100GB worth of weight to the cache
fillCache(10240);
// But nothing has been purged!
for (int i = 0; i < 10000; i++) {
assertNotNull(cache.getIfPresent(i + tenMeg));
}
}
}