You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/06/02 10:45:31 UTC

[GitHub] [iotdb] JackieTien97 commented on a change in pull request #3308: [IOTDB-1415] Fix OOM caused by ChunkCache

JackieTien97 commented on a change in pull request #3308:
URL: https://github.com/apache/iotdb/pull/3308#discussion_r643840023



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/cache/ChunkCache.java
##########
@@ -49,39 +51,64 @@
       config.getAllocateMemoryForChunkCache();
   private static final boolean CACHE_ENABLE = config.isMetaDataCacheEnable();
 
-  private final LRULinkedHashMap<ChunkMetadata, Chunk> lruCache;
+  private final LoadingCache<ChunkMetadata, Chunk> lruCache;
 
-  private final AtomicLong cacheHitNum = new AtomicLong();
-  private final AtomicLong cacheRequestNum = new AtomicLong();
-
-  private final ReadWriteLock lock = new ReentrantReadWriteLock();
+  private final AtomicLong entryAverageSize = new AtomicLong(0);
 
   private ChunkCache() {
     if (CACHE_ENABLE) {
       logger.info("ChunkCache size = " + MEMORY_THRESHOLD_IN_CHUNK_CACHE);
     }
     lruCache =
-        new LRULinkedHashMap<ChunkMetadata, Chunk>(MEMORY_THRESHOLD_IN_CHUNK_CACHE) {
-
-          @Override
-          protected long calEntrySize(ChunkMetadata key, Chunk value) {
-            long currentSize;
-            if (count < 10) {
-              currentSize =
-                  RamUsageEstimator.NUM_BYTES_OBJECT_REF + RamUsageEstimator.sizeOf(value);
-              averageSize = ((averageSize * count) + currentSize) / (++count);
-            } else if (count < 100000) {
-              count++;
-              currentSize = averageSize;
-            } else {
-              averageSize =
-                  RamUsageEstimator.NUM_BYTES_OBJECT_REF + RamUsageEstimator.sizeOf(value);
-              count = 1;
-              currentSize = averageSize;
-            }
-            return currentSize;
-          }
-        };
+        Caffeine.newBuilder()
+            .maximumWeight(MEMORY_THRESHOLD_IN_CHUNK_CACHE)
+            .weigher(
+                new Weigher<ChunkMetadata, Chunk>() {

Review comment:
       It chould be added, but the memory allocated for the cache is fixed, e.g. if we allocate 5MB for cache and even if we can release some part of these 5MB that are not used for a long time, the released memory won't be used by other part of the IoTDB. On the other hand, the expireAfterAccess will cause some overhead.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org