[GitHub] [carbondata] Indhumathi27 opened a new pull request #3653: [WIP]ExpiringMap

[GitBox]

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_r390865833
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
 ##########
 @@ -257,10 +264,22 @@ public void put(TableBlockIndexUniqueIdentifierWrapper tableBlockIndexUniqueIden
         for (BlockDataMap blockletDataMap : dataMaps) {
           blockletDataMap.convertToUnsafeDMStore();
         }
+        // get cacheExpirationTime for table from tableProperties
+        String cacheExpirationTime = null;
+        if (null != tableBlockIndexUniqueIdentifierWrapper.getCarbonTable().getTableInfo()
 
 Review comment:
   When it can be null ? I think null validation is not required.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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_r390879810
 
 

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
 
 Review comment:
   can show command show the expiry time remaining also from cache ?

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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_r390880491
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
 ##########
 @@ -416,6 +416,31 @@ object CommonUtil {
     }
   }
 
+  /**
+   * This method will validate the cache expiration time specified by the user
+   *
+   * @param tableProperties table property specified by user
+   * @param propertyName property name
+   */
+  def validateCacheExpiration(tableProperties: Map[String, String], propertyName: String): Unit = {
+    var expirationTime: java.lang.Long = 0L
+    if (tableProperties.get(propertyName).isDefined) {
+      val value = tableProperties(propertyName)
+      val exceptionMsg = s"Invalid $propertyName value found: " +
+                         s"$value, only duration from 1 minute to 1440 minutes is supported."
+      try {
+        expirationTime = java.lang.Long.parseLong(value)
+      } catch {
+        case n: NumberFormatException =>
+          throw new MalformedCarbonCommandException(exceptionMsg)
+      }
+      if (expirationTime == 0L) {
+        throw new MalformedCarbonCommandException(exceptionMsg)
+      }
+      tableProperties.put(propertyName, value)
 
 Review comment:
   As it is a segment level expiry (because key for cache is mergeindex file name), better to allow alter table properties to change value for new loads.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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_r390878013
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
 ##########
 @@ -2399,4 +2399,9 @@ private CarbonCommonConstants() {
   public static final String BUCKET_COLUMNS = "bucket_columns";
   public static final String BUCKET_NUMBER = "bucket_number";
 
+  /**
+   * property for table level cache expiration
+   */
 
 Review comment:
   Update documentation also

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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_r390877198
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
 ##########
 @@ -2399,4 +2399,9 @@ private CarbonCommonConstants() {
   public static final String BUCKET_COLUMNS = "bucket_columns";
   public static final String BUCKET_NUMBER = "bucket_number";
 
+  /**
+   * property for table level cache expiration
+   */
+  public static final String CACHE_EXPIRATION_TIME = "cache_expiration_time";
+
 
 Review comment:
   Keep a default value of INT_MAX seconds

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 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);
 
 Review comment:
   OK

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
 
 Review comment:
   NO

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
+    Thread.sleep(60000)
 
 Review comment:
   OK

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
+    Thread.sleep(60000)
 
 Review comment:
   OK

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
+    Thread.sleep(60000)
 
 Review comment:
   OK

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 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:
   While creating itself policy is mentioned.. No need to mention again

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
+    Thread.sleep(60000)
 
 Review comment:
   As I have given minutes as timeunit, tested for 1 minute.. I will change time in testcase once I change it to SECOND

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
 ##########
 @@ -416,6 +416,31 @@ object CommonUtil {
     }
   }
 
+  /**
+   * This method will validate the cache expiration time specified by the user
+   *
+   * @param tableProperties table property specified by user
+   * @param propertyName property name
+   */
+  def validateCacheExpiration(tableProperties: Map[String, String], propertyName: String): Unit = {
+    var expirationTime: java.lang.Long = 0L
+    if (tableProperties.get(propertyName).isDefined) {
+      val value = tableProperties(propertyName)
+      val exceptionMsg = s"Invalid $propertyName value found: " +
 
 Review comment:
   OK.. will change in all impacted code.. once I change the timeunit

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
 ##########
 @@ -416,6 +416,31 @@ object CommonUtil {
     }
   }
 
+  /**
+   * This method will validate the cache expiration time specified by the user
+   *
+   * @param tableProperties table property specified by user
+   * @param propertyName property name
+   */
+  def validateCacheExpiration(tableProperties: Map[String, String], propertyName: String): Unit = {
+    var expirationTime: java.lang.Long = 0L
+    if (tableProperties.get(propertyName).isDefined) {
+      val value = tableProperties(propertyName)
+      val exceptionMsg = s"Invalid $propertyName value found: " +
+                         s"$value, only duration from 1 minute to 1440 minutes is supported."
+      try {
+        expirationTime = java.lang.Long.parseLong(value)
+      } catch {
+        case n: NumberFormatException =>
+          throw new MalformedCarbonCommandException(exceptionMsg)
+      }
+      if (expirationTime == 0L) {
+        throw new MalformedCarbonCommandException(exceptionMsg)
+      }
+      tableProperties.put(propertyName, value)
 
 Review comment:
   It is currently added as a table-level segment expiry, means, expiration is for table level cache. Alter table properties is still needed?

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/cache/CarbonLRUCache.java
 ##########
 @@ -95,9 +97,9 @@ public CarbonLRUCache(String propertyName, String defaultPropertyName) {
    * initialize lru cache
    */
   private void initCache() {
-    lruCacheMap =
-        new LinkedHashMap<String, Cacheable>(CarbonCommonConstants.DEFAULT_COLLECTION_SIZE, 1.0f,
-            true);
+    expiringMap =
+        ExpiringMap.builder().expirationPolicy(ExpirationPolicy.ACCESSED).variableExpiration()
 
 Review comment:
   done

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
 ##########
 @@ -2399,4 +2399,9 @@ private CarbonCommonConstants() {
   public static final String BUCKET_COLUMNS = "bucket_columns";
   public static final String BUCKET_NUMBER = "bucket_number";
 
+  /**
+   * property for table level cache expiration
+   */
+  public static final String CACHE_EXPIRATION_TIME = "cache_expiration_time";
 
 Review comment:
   done

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
 ##########
 @@ -257,10 +264,22 @@ public void put(TableBlockIndexUniqueIdentifierWrapper tableBlockIndexUniqueIden
         for (BlockDataMap blockletDataMap : dataMaps) {
           blockletDataMap.convertToUnsafeDMStore();
         }
+        // get cacheExpirationTime for table from tableProperties
+        String cacheExpirationTime = null;
+        if (null != tableBlockIndexUniqueIdentifierWrapper.getCarbonTable().getTableInfo()
 
 Review comment:
   removed

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/BlockletDataMapIndexStore.java
 ##########
 @@ -146,8 +146,15 @@ public BlockletDataMapIndexWrapper get(TableBlockIndexUniqueIdentifierWrapper id
               new BlockletDataMapIndexWrapper(identifier.getSegmentId(), dataMaps);
         }
         if (identifierWrapper.isAddTableBlockToUnsafeAndLRUCache()) {
+          String cacheExpirationTime =
+              identifierWrapper.getCarbonTable().getTableInfo().getFactTable().getTableProperties()
 
 Review comment:
   done

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
 ##########
 @@ -416,6 +416,31 @@ object CommonUtil {
     }
   }
 
+  /**
+   * This method will validate the cache expiration time specified by the user
+   *
+   * @param tableProperties table property specified by user
+   * @param propertyName property name
+   */
+  def validateCacheExpiration(tableProperties: Map[String, String], propertyName: String): Unit = {
+    var expirationTime: java.lang.Long = 0L
+    if (tableProperties.get(propertyName).isDefined) {
+      val value = tableProperties(propertyName)
+      val exceptionMsg = s"Invalid $propertyName value found: " +
 
 Review comment:
   changed

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

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

 ##########
 File path: integration/spark/src/test/scala/org/apache/carbondata/sql/commands/TestCarbonShowCacheCommand.scala
 ##########
 @@ -288,4 +289,25 @@ class TestCarbonShowCacheCommand extends QueryTest with BeforeAndAfterAll {
     assert(showCache(0).get(2).toString.equalsIgnoreCase("5/5 index files cached"))
     sql("drop table if exists partitionTable")
   }
+
+  test("test cache expiration using expiringMap") {
+    sql("drop table if exists carbonTable")
+    sql("create table carbonTable(col1 int, col2 string,col3 string) stored as carbondata tblproperties('cache_expiration_time'='1')")
+    sql("insert into carbonTable select 1, 'ab', 'vf'")
+    sql("select count(*) from carbonTable").show(false)
+    var showCache = sql("show metacache on table carbonTable").collect()
+    assert(showCache(0).get(2).toString.equalsIgnoreCase("1/1 index files cached"))
+    Thread.sleep(60000)
 
 Review comment:
   changed

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services