From 7bc5ab45bc5336478d482e2e19a510fd50fb61f8 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 2 Apr 2014 16:28:51 +0200 Subject: [PATCH] Cleanup IndicesFieldDataCache and IndexFieldDataCache This commit adds several asserts and removes possible `null` values from the `FieldDataCache` implementation. Closes #5664 --- .../index/fielddata/IndexFieldDataCache.java | 26 +++++---- .../cache/IndicesFieldDataCache.java | 53 ++++++++----------- 2 files changed, 34 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataCache.java b/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataCache.java index f7f00f9ddd5..6d36c302f91 100644 --- a/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataCache.java +++ b/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataCache.java @@ -68,14 +68,14 @@ public interface IndexFieldDataCache { * The resident field data cache is a *per field* cache that keeps all the values in memory. */ static abstract class FieldBased implements IndexFieldDataCache, SegmentReader.CoreClosedListener, RemovalListener { - @Nullable private final IndexService indexService; private final FieldMapper.Names fieldNames; private final FieldDataType fieldDataType; private final Cache cache; private final IndicesFieldDataCacheListener indicesFieldDataCacheListener; - protected FieldBased(@Nullable IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, CacheBuilder cache, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { + protected FieldBased(IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, CacheBuilder cache, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { + assert indexService != null; this.indexService = indexService; this.fieldNames = fieldNames; this.fieldDataType = fieldDataType; @@ -87,11 +87,12 @@ public interface IndexFieldDataCache { @Override public void onRemoval(RemovalNotification notification) { - Key key = notification.getKey(); + final Key key = notification.getKey(); assert key != null && key.listeners != null; - AtomicFieldData value = notification.getValue(); + final AtomicFieldData value = notification.getValue(); long sizeInBytes = key.sizeInBytes; + assert sizeInBytes >= 0 || value != null : "Expected size [" + sizeInBytes + "] to be positive or value [" + value + "] to be non-null"; if (sizeInBytes == -1 && value != null) { sizeInBytes = value.getMemorySizeInBytes(); } @@ -111,14 +112,11 @@ public interface IndexFieldDataCache { AtomicFieldData fieldData = indexFieldData.loadDirect(context); key.sizeInBytes = fieldData.getMemorySizeInBytes(); key.listeners.add(indicesFieldDataCacheListener); - - if (indexService != null) { - ShardId shardId = ShardUtils.extractShardId(context.reader()); - if (shardId != null) { - IndexShard shard = indexService.shard(shardId.id()); - if (shard != null) { - key.listeners.add(shard.fieldData()); - } + final ShardId shardId = ShardUtils.extractShardId(context.reader()); + if (shardId != null) { + final IndexShard shard = indexService.shard(shardId.id()); + if (shard != null) { + key.listeners.add(shard.fieldData()); } } for (Listener listener : key.listeners) { @@ -175,14 +173,14 @@ public interface IndexFieldDataCache { static class Resident extends FieldBased { - public Resident(@Nullable IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { + public Resident(IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { super(indexService, fieldNames, fieldDataType, CacheBuilder.newBuilder(), indicesFieldDataCacheListener); } } static class Soft extends FieldBased { - public Soft(@Nullable IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { + public Soft(IndexService indexService, FieldMapper.Names fieldNames, FieldDataType fieldDataType, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { super(indexService, fieldNames, fieldDataType, CacheBuilder.newBuilder().softValues(), indicesFieldDataCacheListener); } } diff --git a/src/main/java/org/elasticsearch/indices/fielddata/cache/IndicesFieldDataCache.java b/src/main/java/org/elasticsearch/indices/fielddata/cache/IndicesFieldDataCache.java index 3c4456ad10e..b2910219a8d 100644 --- a/src/main/java/org/elasticsearch/indices/fielddata/cache/IndicesFieldDataCache.java +++ b/src/main/java/org/elasticsearch/indices/fielddata/cache/IndicesFieldDataCache.java @@ -22,7 +22,6 @@ package org.elasticsearch.indices.fielddata.cache; import com.google.common.cache.*; import org.apache.lucene.index.AtomicReaderContext; import org.apache.lucene.index.SegmentReader; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.lucene.SegmentReaderUtils; @@ -50,25 +49,15 @@ import java.util.concurrent.TimeUnit; public class IndicesFieldDataCache extends AbstractComponent implements RemovalListener { private final IndicesFieldDataCacheListener indicesFieldDataCacheListener; - - Cache cache; - - private volatile String size; - private volatile long sizeInBytes; - private volatile TimeValue expire; - + private final Cache cache; @Inject public IndicesFieldDataCache(Settings settings, IndicesFieldDataCacheListener indicesFieldDataCacheListener) { super(settings); this.indicesFieldDataCacheListener = indicesFieldDataCacheListener; - this.size = componentSettings.get("size", "-1"); - this.sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes(); - this.expire = componentSettings.getAsTime("expire", null); - buildCache(); - } - - private void buildCache() { + final String size = componentSettings.get("size", "-1"); + final long sizeInBytes = componentSettings.getAsMemory("size", "-1").bytes(); + final TimeValue expire = componentSettings.getAsTime("expire", null); CacheBuilder cacheBuilder = CacheBuilder.newBuilder() .removalListener(this); if (sizeInBytes > 0) { @@ -87,18 +76,18 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL cache.invalidateAll(); } - public IndexFieldDataCache buildIndexFieldDataCache(@Nullable IndexService indexService, Index index, FieldMapper.Names fieldNames, FieldDataType fieldDataType) { - return new IndexFieldCache(indexService, index, fieldNames, fieldDataType); + public IndexFieldDataCache buildIndexFieldDataCache(IndexService indexService, Index index, FieldMapper.Names fieldNames, FieldDataType fieldDataType) { + return new IndexFieldCache(cache, indicesFieldDataCacheListener, indexService, index, fieldNames, fieldDataType); } @Override public void onRemoval(RemovalNotification notification) { - Key key = notification.getKey(); + final Key key = notification.getKey(); assert key != null && key.listeners != null; - IndexFieldCache indexCache = key.indexCache; long sizeInBytes = key.sizeInBytes; - AtomicFieldData value = notification.getValue(); + final AtomicFieldData value = notification.getValue(); + assert sizeInBytes >= 0 || value != null : "Expected size [" + sizeInBytes + "] to be positive or value [" + value + "] to be non-null"; if (sizeInBytes == -1 && value != null) { sizeInBytes = value.getMemorySizeInBytes(); } @@ -119,19 +108,23 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL /** * A specific cache instance for the relevant parameters of it (index, fieldNames, fieldType). */ - class IndexFieldCache implements IndexFieldDataCache, SegmentReader.CoreClosedListener { + static class IndexFieldCache implements IndexFieldDataCache, SegmentReader.CoreClosedListener { - @Nullable private final IndexService indexService; final Index index; final FieldMapper.Names fieldNames; final FieldDataType fieldDataType; + private final Cache cache; + private final IndicesFieldDataCacheListener indicesFieldDataCacheListener; - IndexFieldCache(@Nullable IndexService indexService, Index index, FieldMapper.Names fieldNames, FieldDataType fieldDataType) { + IndexFieldCache(final Cache cache, IndicesFieldDataCacheListener indicesFieldDataCacheListener, IndexService indexService, Index index, FieldMapper.Names fieldNames, FieldDataType fieldDataType) { this.indexService = indexService; this.index = index; this.fieldNames = fieldNames; this.fieldDataType = fieldDataType; + this.cache = cache; + this.indicesFieldDataCacheListener = indicesFieldDataCacheListener; + assert indexService != null; } @Override @@ -143,15 +136,13 @@ public class IndicesFieldDataCache extends AbstractComponent implements RemovalL public AtomicFieldData call() throws Exception { SegmentReaderUtils.registerCoreListener(context.reader(), IndexFieldCache.this); AtomicFieldData fieldData = indexFieldData.loadDirect(context); + key.sizeInBytes = fieldData.getMemorySizeInBytes(); key.listeners.add(indicesFieldDataCacheListener); - - if (indexService != null) { - ShardId shardId = ShardUtils.extractShardId(context.reader()); - if (shardId != null) { - IndexShard shard = indexService.shard(shardId.id()); - if (shard != null) { - key.listeners.add(shard.fieldData()); - } + final ShardId shardId = ShardUtils.extractShardId(context.reader()); + if (shardId != null) { + final IndexShard shard = indexService.shard(shardId.id()); + if (shard != null) { + key.listeners.add(shard.fieldData()); } } for (Listener listener : key.listeners) {