mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-02-17 18:35:25 +00:00
[CORE] Remove explicit .cleanUp() on cache clear
Calling cache.cleanUp() is kind of like calling System.gc(), meaning that we should never have (non-test) things that rely on this functionality. For the field data and filter cache, we already have a periodic process that runs this .cleanUp(), so there is no need to block index closing/clearing on it. Instead, we can clean the field data cache in InternalTestCluster before we check the circuit breaker. This can help tests that time out because cleaning the cache is taking too long
This commit is contained in:
parent
42d9a57d0c
commit
6bf18056b0
@ -117,9 +117,8 @@ public class BitsetFilterCache extends AbstractIndexComponent implements LeafRea
|
||||
}
|
||||
|
||||
public void clear(String reason) {
|
||||
logger.debug("Clearing all Bitsets because [{}]", reason);
|
||||
logger.debug("clearing all bitsets because [{}]", reason);
|
||||
loadedFilters.invalidateAll();
|
||||
loadedFilters.cleanUp();
|
||||
}
|
||||
|
||||
private BitDocIdSet getAndLoadIfNotPresent(final Filter filter, final LeafReaderContext context) throws IOException, ExecutionException {
|
||||
|
@ -154,7 +154,6 @@ public class IndicesFilterCache extends AbstractComponent implements RemovalList
|
||||
public void close() {
|
||||
closed = true;
|
||||
cache.invalidateAll();
|
||||
cache.cleanUp();
|
||||
}
|
||||
|
||||
public Cache<WeightedFilterCache.FilterCacheKey, DocIdSet> cache() {
|
||||
|
@ -154,13 +154,6 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL
|
||||
assert indexService != null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up the internal Guava cache
|
||||
*/
|
||||
public void cleanUp() {
|
||||
cache.cleanUp();
|
||||
}
|
||||
|
||||
@Override
|
||||
public <FD extends AtomicFieldData, IFD extends IndexFieldData<FD>> FD load(final LeafReaderContext context, final IFD indexFieldData) throws Exception {
|
||||
final Key key = new Key(this, context.reader().getCoreCacheKey());
|
||||
@ -235,24 +228,18 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL
|
||||
|
||||
@Override
|
||||
public void clear() {
|
||||
// TODO: determine whether there is ever anything in this cache that doesn't share the index and consider .invalidateAll() instead
|
||||
// Note that cache invalidation in Guava does not immediately remove
|
||||
// values from the cache. In the case of a cache with a rare write or
|
||||
// read rate, it's possible for values to persist longer than desired.
|
||||
//
|
||||
// Note this is intended by the Guava developers, see:
|
||||
// https://code.google.com/p/guava-libraries/wiki/CachesExplained#Eviction
|
||||
// (the "When Does Cleanup Happen" section)
|
||||
for (Key key : cache.asMap().keySet()) {
|
||||
if (key.indexCache.index.equals(index)) {
|
||||
cache.invalidate(key);
|
||||
}
|
||||
}
|
||||
// There is an explicit call to cache.cleanUp() here because cache
|
||||
// invalidation in Guava does not immediately remove values from the
|
||||
// cache. In the case of a cache with a rare write or read rate,
|
||||
// it's possible for values to persist longer than desired. In the
|
||||
// case of the circuit breaker, when clearing the entire cache all
|
||||
// entries should immediately be evicted so that their sizes are
|
||||
// removed from the breaker estimates.
|
||||
//
|
||||
// Note this is intended by the Guava developers, see:
|
||||
// https://code.google.com/p/guava-libraries/wiki/CachesExplained#Eviction
|
||||
// (the "When Does Cleanup Happen" section)
|
||||
cache.cleanUp();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -29,7 +29,6 @@ import com.google.common.collect.*;
|
||||
import com.google.common.util.concurrent.Futures;
|
||||
import com.google.common.util.concurrent.ListenableFuture;
|
||||
import com.google.common.util.concurrent.SettableFuture;
|
||||
|
||||
import org.apache.lucene.util.AbstractRandomizedTest;
|
||||
import org.apache.lucene.util.IOUtils;
|
||||
import org.elasticsearch.ElasticsearchException;
|
||||
@ -70,15 +69,16 @@ import org.elasticsearch.common.util.BigArraysModule;
|
||||
import org.elasticsearch.common.util.concurrent.EsExecutors;
|
||||
import org.elasticsearch.env.NodeEnvironment;
|
||||
import org.elasticsearch.http.HttpServerTransport;
|
||||
import org.elasticsearch.index.IndexService;
|
||||
import org.elasticsearch.index.cache.filter.FilterCacheModule;
|
||||
import org.elasticsearch.index.cache.filter.none.NoneFilterCache;
|
||||
import org.elasticsearch.index.cache.filter.weighted.WeightedFilterCache;
|
||||
import org.elasticsearch.index.engine.IndexEngineModule;
|
||||
import org.elasticsearch.index.IndexService;
|
||||
import org.elasticsearch.index.shard.ShardId;
|
||||
import org.elasticsearch.indices.IndicesService;
|
||||
import org.elasticsearch.indices.breaker.CircuitBreakerService;
|
||||
import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;
|
||||
import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
|
||||
import org.elasticsearch.indices.recovery.RecoverySettings;
|
||||
import org.elasticsearch.node.Node;
|
||||
import org.elasticsearch.node.internal.InternalNode;
|
||||
@ -111,15 +111,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
import static junit.framework.Assert.fail;
|
||||
import static org.apache.lucene.util.LuceneTestCase.TEST_NIGHTLY;
|
||||
import static org.apache.lucene.util.LuceneTestCase.rarely;
|
||||
import static org.apache.lucene.util.LuceneTestCase.usually;
|
||||
import static org.apache.lucene.util.LuceneTestCase.*;
|
||||
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
|
||||
import static org.elasticsearch.node.NodeBuilder.nodeBuilder;
|
||||
import static org.elasticsearch.test.ElasticsearchTestCase.assertBusy;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertThat;
|
||||
|
||||
@ -1693,6 +1690,10 @@ public final class InternalTestCluster extends TestCluster {
|
||||
// network request, because a network request can increment one
|
||||
// of the breakers
|
||||
for (NodeAndClient nodeAndClient : nodes.values()) {
|
||||
final IndicesFieldDataCache fdCache = getInstanceFromNode(IndicesFieldDataCache.class, nodeAndClient.node);
|
||||
// Clean up the cache, ensuring that entries' listeners have been called
|
||||
fdCache.getCache().cleanUp();
|
||||
|
||||
final String name = nodeAndClient.name;
|
||||
final CircuitBreakerService breakerService = getInstanceFromNode(CircuitBreakerService.class, nodeAndClient.node);
|
||||
CircuitBreaker fdBreaker = breakerService.getBreaker(CircuitBreaker.Name.FIELDDATA);
|
||||
|
Loading…
x
Reference in New Issue
Block a user