wchevreuil commented on code in PR #6439:
URL: https://github.com/apache/hbase/pull/6439#discussion_r1836847469
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/RSMobFileCleanerChore.java:
##########
@@ -116,14 +120,32 @@ protected void chore() {
Set<String> regionMobs = new HashSet<String>();
Path currentPath = null;
try {
- // collectinng referenced MOBs
+ // collecting referenced MOBs
for (HStoreFile sf : sfs) {
currentPath = sf.getPath();
- sf.initReader();
- byte[] mobRefData =
sf.getMetadataValue(HStoreFile.MOB_FILE_REFS);
- byte[] bulkloadMarkerData =
sf.getMetadataValue(HStoreFile.BULKLOAD_TASK_KEY);
- // close store file to avoid memory leaks
- sf.closeStoreFile(true);
+ byte[] mobRefData = null;
+ byte[] bulkloadMarkerData = null;
+ boolean needCreateReader = false;
+ if (sf.getReader() == null) {
+ synchronized (sf) {
+ if (sf.getReader() == null) {
+ needCreateReader = true;
+ sf.initReader();
+ mobRefData =
sf.getMetadataValue(HStoreFile.MOB_FILE_REFS);
+ bulkloadMarkerData =
sf.getMetadataValue(HStoreFile.BULKLOAD_TASK_KEY);
+ // close store file to avoid memory leaks
+ sf.closeStoreFile(true);
+ }
+ }
+ }
+ // If the StoreFileReader object was created by another
thread, even if the reader
+ // has been closed now, we can still obtain the data by
HStoreFile.metataMap,
+ // because the map will not be cleared when the reader is
closed.
+ if (!needCreateReader) {
+ mobRefData = sf.getMetadataValue(HStoreFile.MOB_FILE_REFS);
+ bulkloadMarkerData =
sf.getMetadataValue(HStoreFile.BULKLOAD_TASK_KEY);
+ }
Review Comment:
I guess the operations within the synchronized block are all O(1), apart
from close when we have to evict blocks from the cache, but we would only call
close here if the reader was null, which means, this file has probably been
compacted already and have no references to it. Now, in such cases when reader
was null, would we ever open a reader again outside this cleaner chore? I
suppose not.
Nevertheless, this is a minor style suggestion, am ok to go without it.
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
