keith-turner commented on code in PR #4551:
URL: https://github.com/apache/accumulo/pull/4551#discussion_r1597490236
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java:
##########
@@ -246,9 +246,30 @@ public ScanServer(ScanServerOpts opts, String[] args) {
LOG.warn(
"Tablet metadata caching less than one minute, may cause excessive
scans on metadata table.");
}
- tabletMetadataCache =
- Caffeine.newBuilder().expireAfterWrite(cacheExpiration,
TimeUnit.MILLISECONDS)
-
.scheduler(Scheduler.systemScheduler()).recordStats().build(tabletMetadataLoader);
+
+ // Get the cache refresh percentage property
+ // Value must be less than 100% as 100 or over would effectively disable
it
+ double cacheRefreshPercentage =
+
getConfiguration().getFraction(Property.SSERV_CACHED_TABLET_METADATA_REFRESH_PERCENT);
+ Preconditions.checkArgument(cacheRefreshPercentage < cacheExpiration,
+ "Tablet metadata cache refresh percentage is '%s' but must be less
than 1",
+ cacheRefreshPercentage);
+
+ var builder = Caffeine.newBuilder().expireAfterWrite(cacheExpiration,
TimeUnit.MILLISECONDS)
+ .scheduler(Scheduler.systemScheduler()).recordStats();
+ if (cacheRefreshPercentage > 0) {
+ // Compute the refresh time as a percentage of the expiration time
+ // Cache hits after this time, but before expiration, will trigger a
background
+ // non-blocking refresh of the entry so future cache hits get an
updated entry
+ // without having to block for a refresh
+ long cacheRefresh = (long) (cacheExpiration * cacheRefreshPercentage);
+ LOG.debug("Tablet metadata refresh percentage set to {}, refresh time
set to {} ms",
+ cacheRefreshPercentage, cacheRefresh);
+ builder.refreshAfterWrite(cacheRefresh, TimeUnit.MILLISECONDS);
Review Comment:
What thread will run the refresh operation? Based on comments by @ben-manes
on #4544 it seems like the JVM wide thread pool may be used, which may not be
best for I/O heavy refresh that scan server will do. If that is the case may
want to use a thread pool created in the scan server for refresh.
Looking at the javadocs, when an error happens in a background refresh it
seems like Caffeine will log this somewhere possibly using JVM logging APIs.
Not sure where that would go. If these will not go to slf4j then we may want
to have these background refresh task try/catch/log into a slf4j logger we know
about.
Was unsure if this code should be `builder =
builder.refreshAfterWrite(...)`. Looked at the javadoc and it mentions `this`
is returned, so do not need to do that.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
