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:
[email protected]
With regards,
Apache Git Services
