[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3653: [CARBONDATA-3665] Support TimeBased Cache expiration using ExpiringMap

[GitBox <gi...@apache.org>]

ajantha-bhat commented on a change in pull request #3653: [CARBONDATA-3665] Support TimeBased Cache expiration using ExpiringMap
URL: https://github.com/apache/carbondata/pull/3653#discussion_r390856996
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/cache/CarbonLRUCache.java
 ##########
 @@ -215,54 +218,27 @@ public boolean put(String columnIdentifier, Cacheable cacheInfo, long requiredSi
         }
       }
     } else {
-      synchronized (lruCacheMap) {
-        addEntryToLRUCacheMap(columnIdentifier, cacheInfo);
+      synchronized (expiringMap) {
+        addEntryToLRUCacheMap(columnIdentifier, cacheInfo, expiration_time);
         currentSize = currentSize + requiredSize;
       }
       columnKeyAddedSuccessfully = true;
     }
     return columnKeyAddedSuccessfully;
   }
 
-  /**
-   * This method will check if required size is available in the memory
-   * @param columnIdentifier
-   * @param requiredSize
-   * @return
-   */
-  public boolean tryPut(String columnIdentifier, long requiredSize) {
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("checking Required size for entry " + columnIdentifier + " :: " + requiredSize
-          + " Current cache size :: " + currentSize);
-    }
-    boolean columnKeyCanBeAdded = false;
-    if (isLRUCacheSizeConfigured()) {
-      synchronized (lruCacheMap) {
-        if (freeMemorySizeForAddingCache(requiredSize)) {
-          columnKeyCanBeAdded = true;
-        } else {
-          LOGGER.error(
-              "Size check failed.Size not available. Entry cannot be added to lru cache :: "
-                  + columnIdentifier + " .Required Size = " + requiredSize + " Size available " + (
-                  lruCacheMemorySize - currentSize));
-        }
-      }
-    } else {
-      columnKeyCanBeAdded = true;
-    }
-    return columnKeyCanBeAdded;
-  }
-
   /**
    * The method will add the cache entry to LRU cache map
    *
    * @param columnIdentifier
    * @param cacheInfo
    */
-  private void addEntryToLRUCacheMap(String columnIdentifier, Cacheable cacheInfo) {
-    if (null == lruCacheMap.get(columnIdentifier)) {
-      lruCacheMap.put(columnIdentifier, cacheInfo);
-    }
+  private void addEntryToLRUCacheMap(String columnIdentifier, Cacheable cacheInfo,
+      long expiration_time) {
+    if (null == expiringMap.get(columnIdentifier) && expiration_time != 0L) {
+      expiringMap.put(columnIdentifier, cacheInfo, ExpirationPolicy.ACCESSED, expiration_time,
+          TimeUnit.MINUTES);
+    } else expiringMap.putIfAbsent(columnIdentifier, cacheInfo);
 
 Review comment:
   Need to assign a default value and keep ExpirationPolicy.ACCESSED as policy.
   
   Also maintain {} for else

----------------------------------------------------------------
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


With regards,
Apache Git Services