-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-27795: Define RPC API for cache cleaning #5492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,9 +58,12 @@ | |
| import java.util.TimerTask; | ||
| import java.util.TreeMap; | ||
| import java.util.TreeSet; | ||
| import java.util.concurrent.Callable; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.ConcurrentMap; | ||
| import java.util.concurrent.ConcurrentSkipListMap; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ThreadLocalRandom; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
@@ -1659,6 +1662,43 @@ public RegionLoad createRegionLoad(final String encodedRegionName) throws IOExce | |
| return r != null ? createRegionLoad(r, null, null) : null; | ||
| } | ||
|
|
||
| public Map<String, Integer> uncacheStaleBlocks() { | ||
| Map<String, Pair<String, Long>> fullyCachedFiles = | ||
| this.getBlockCache().flatMap(BlockCache::getFullyCachedFiles).orElse(Collections.emptyMap()); | ||
| Map<String, Integer> evictedFilesWithStaleBlocks = new ConcurrentHashMap<>(); | ||
|
|
||
| ExecutorService executor = Executors.newFixedThreadPool(6); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The threadpool should be global for HRegionServer. We don't want a new threadpool created on every client call to this method, that would potentially kill the RS if an operator calls it multiple times on a short period. |
||
|
|
||
| List<Callable<Void>> tasks = new ArrayList<>(); | ||
|
|
||
| fullyCachedFiles.forEach((fileName, value) -> { | ||
| Callable<Void> task = () -> { | ||
| HRegion regionOnServer = getRegion(value.getFirst()); | ||
| int blocksEvicted = (regionOnServer == null || !regionOnServer.isAvailable()) | ||
| ? this.getBlockCache().get().evictBlocksByHfileName(fileName) | ||
| : 0; | ||
| evictedFilesWithStaleBlocks.put(fileName, blocksEvicted); | ||
| LOG.info( | ||
| "Uncached {} blocks belonging to the file {} as the region {} " | ||
| + "is not served by the region server {} anymore.", | ||
| blocksEvicted, fileName, value.getFirst(), this.getServerName()); | ||
| return null; | ||
| }; | ||
| tasks.add(task); | ||
|
Comment on lines
+1675
to
+1687
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do a single task per call to the method, not a task per file, that would create yet another collection in memory with as many objects as the total files cached. On large caches, that would be impacting. |
||
| }); | ||
|
|
||
| try { | ||
| executor.invokeAll(tasks); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| LOG.error("Thread interrupted while processing tasks for uncaching stale blocks: {}", | ||
| e.getMessage()); | ||
| } finally { | ||
| executor.shutdown(); | ||
| } | ||
| return evictedFilesWithStaleBlocks; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should think about an alternative result here, as this map is effectively being updated by the background task, which can be confusing for the caller. I would rather make this as void, or a boolean, returning true indicating that the task is submitted and is running in the background. |
||
| } | ||
|
|
||
| /** | ||
| * Inner class that runs on a long period checking if regions need compaction. | ||
| */ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.