wchevreuil commented on code in PR #8469:
URL: https://github.com/apache/hbase/pull/8469#discussion_r3586703592
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -1798,70 +1829,110 @@ private void checkIOErrorIsTolerated() {
if (isCacheEnabled() && (now - ioErrorStartTimeTmp) >
this.ioErrorsTolerationDuration) {
LOG.error("IO errors duration time has exceeded " +
ioErrorsTolerationDuration
+ "ms, disabling cache, please check your IOEngine");
- disableCache();
+ disableCache(false);
}
} else {
this.ioErrorStartTime = now;
}
}
/**
- * Used to shut down the cache -or- turn it off in the case of something
broken.
+ * Used to shut down the cache -or- turn it off in the case of something
broken. Cleanup runs
+ * separately because IO errors can invoke this method from a writer thread
or while holding an
+ * offset read lock.
*/
- private void disableCache() {
+ private synchronized void disableCache(boolean persistOnCleanup) {
if (!isCacheEnabled()) {
return;
}
LOG.info("Disabling cache");
cacheState = CacheState.DISABLED;
ioEngine.shutdown();
this.scheduleThreadPool.shutdown();
- for (int i = 0; i < writerThreads.length; ++i)
- writerThreads[i].interrupt();
- this.ramCache.clear();
- if (!ioEngine.isPersistent() || persistencePath == null) {
- // If persistent ioengine and a path, we will serialize out the
backingMap.
- this.backingMap.clear();
- this.blocksByHFile.clear();
- this.fullyCachedFiles.clear();
- this.regionCachedSize.clear();
+ for (WriterThread writerThread : writerThreads) {
+ writerThread.interrupt();
}
+ ramCache.clear();
if (cacheStats.getMetricsRollerScheduler() != null) {
cacheStats.getMetricsRollerScheduler().shutdownNow();
}
+ cacheCleanupThread =
+ Threads.setDaemonThreadRunning(new Thread(() ->
cleanupCache(persistOnCleanup)),
+ "BucketCacheCleanup-" + System.identityHashCode(this),
Threads.LOGGING_EXCEPTION_HANDLER);
}
- private void join() throws InterruptedException {
- for (int i = 0; i < writerThreads.length; ++i)
- writerThreads[i].join();
- }
-
- @Override
- public void shutdown() {
- if (isCacheEnabled()) {
- disableCache();
- LOG.info("Shutdown bucket cache: IO persistent=" +
ioEngine.isPersistent()
- + "; path to write=" + persistencePath);
- if (ioEngine.isPersistent() && persistencePath != null) {
+ private void cleanupCache(boolean persistOnCleanup) {
Review Comment:
nit: Let's call this 'cleanupBackingMap' or 'cleanupCacheIndex' to avoid
confusion.
--
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]
