[GitHub] keith-turner commented on a change in pull request #956: FLUO-939 Make TransactorCache configurable

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

keith-turner commented on a change in pull request #956: FLUO-939 Make TransactorCache configurable
URL: https://github.com/apache/fluo/pull/956#discussion_r146926301
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/fluo/core/impl/FluoConfigurationImpl.java
 ##########
 @@ -185,6 +185,52 @@ public static long getVisibilityCacheTimeout(FluoConfiguration conf, TimeUnit tu
     return tu.convert(millis, TimeUnit.MILLISECONDS);
   }
 
+  private static final String TRANSACTOR_MAX_CACHE_SIZE =
+      FLUO_IMPL_PREFIX + ".transactor.cache.max.size";
 
 Review comment:
   This config setting is controlling the max number of key values to cache, not the sum of the size of the key values.  I thought it was the sum at first glance, but realized it was not on closer inspection. 
   
   I found this size vs weight terminology with the config and cache API confusing. To make the config more clear, I think we should use the term `weight` on the props that set weight in the guava API.  For example change `.visibility.cache.size.mb` to `.visibility.cache.weight.mb`.  I think this change would make sense as a follow on issue, but it could also be done in this PR.
   
   For this property we should keep using size because it corresponds with the Guava API. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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