phoenix-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] dbwong commented on a change in pull request #425: PHOENIX-5069 Use asynchronous refresh to provide non-blocking Phoenix Stats Client Cache
Date Wed, 16 Jan 2019 22:46:13 GMT
dbwong commented on a change in pull request #425: PHOENIX-5069 Use asynchronous refresh to
provide non-blocking Phoenix Stats Client Cache
URL: https://github.com/apache/phoenix/pull/425#discussion_r248472913
 
 

 ##########
 File path: phoenix-core/src/main/java/org/apache/phoenix/query/GuidePostsCache.java
 ##########
 @@ -59,22 +59,43 @@
 
     private final ConnectionQueryServices queryServices;
     private final LoadingCache<GuidePostsKey, GuidePostsInfo> cache;
+    private ExecutorService executor = null;
 
     public GuidePostsCache(ConnectionQueryServices queryServices, Configuration config) {
         this.queryServices = Objects.requireNonNull(queryServices);
+
         // Number of millis to expire cache values after write
         final long statsUpdateFrequency = config.getLong(
                 QueryServices.STATS_UPDATE_FREQ_MS_ATTRIB,
                 QueryServicesOptions.DEFAULT_STATS_UPDATE_FREQ_MS);
-        // Maximum number of entries (tables) to store in the cache at one time
+
+        // Maximum total weight (size in bytes) of stats entries
         final long maxTableStatsCacheSize = config.getLong(
                 QueryServices.STATS_MAX_CACHE_SIZE,
                 QueryServicesOptions.DEFAULT_STATS_MAX_CACHE_SIZE);
+
 		final boolean isStatsEnabled = config.getBoolean(STATS_COLLECTION_ENABLED, DEFAULT_STATS_COLLECTION_ENABLED)
 				&& config.getBoolean(STATS_ENABLED_ATTRIB, true);
+
+        PhoenixStatsCacheLoader cacheLoader;
+
+        if (isStatsEnabled) {
+            // The size of the thread pool used for refreshing cached table stats
+            final int statsCacheThreadPoolSize = config.getInt(
+                    QueryServices.STATS_CACHE_THREAD_POOL_SIZE,
+                    QueryServicesOptions.DEFAULT_STATS_CACHE_THREAD_POOL_SIZE);
+
+            executor = Executors.newFixedThreadPool(statsCacheThreadPoolSize);
 
 Review comment:
   Why was the executorService placed at this level?  Why do we need a private variable we
don't use it outside of this one place.  Did we consider pushing it to a place where we can
be more assured of it being constructed only once and then providing an accessor that the
CacheLoader can get at?  I'm assuming we only want one instance of the GuidePostsCache but
the constructor is public and the executor service isn't static and protected.

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

Mime
View raw message