BinShi-SecularBird 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_r249141342
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/query/GuidePostsCache.java
##########
@@ -128,20 +164,30 @@ public GuidePostsInfo load(GuidePostsKey statsKey)
throws Exception {
void traceStatsUpdate(GuidePostsKey key, GuidePostsInfo info) {
if (logger.isTraceEnabled()) {
logger.trace("Updating local TableStats cache (id={}) for {},
size={}bytes",
- new Object[] {Objects.hashCode(GuidePostsCache.this),
key,
- info.getEstimatedSize()});
+ new Object[] {Objects.hashCode(GuidePostsCache.this),
key, info.getEstimatedSize()});
}
}
}
/**
+ * {@link PhoenixStatsLoader} implementation for the Stats Loader.
* Empty stats loader if stats are disabled
*/
- protected class EmptyStatsLoader extends CacheLoader<GuidePostsKey,
GuidePostsInfo> {
- @Override
- public GuidePostsInfo load(GuidePostsKey statsKey) throws
Exception {
- return GuidePostsInfo.NO_GUIDEPOST;
- }
+ protected class EmptyStatsLoader implements PhoenixStatsLoader {
Review comment:
I don't like it too. It was introduced by Open Source Community
"PHOENIX-5010 (https://issues.apache.org/jira/browse/PHOENIX-5010) Don't build
client guidepost cache when phoenix.stats.collection.enabled is disabled"
instead of my change.
There is a discussion between Karan and me, and my opinion is "my thought is
that this is fastest and simplest way to fix it, although it isn't the most
efficient way. Fixing in this way provides the uniform interface to the callers
of the GuidePostCache so the calls don't need to something like "if stats are
enabled then call this; otherwise call that". **One of efficient and formal
ways to fix it is that we define the interface of GuidePostsCache, two classes
implement the interface - one is GuidePostsEmptyCache and another is current
GuidePostsCache. We create object of GuidePostsEmptyCache when stats are
disabled.**
**It is out of the scope of this change and we can reconsider it when we
implement JVM based cache**."
In the formal solution of PHOENIX-5010 that I mentioned above, the
GuidePostsEmptyCache is still something of "do nothing class" which you don't
like, but I like it because it keeps the interface and its usage to be simple
and clean which is important to me.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
