guluo2016 commented on code in PR #6439:
URL: https://github.com/apache/hbase/pull/6439#discussion_r1828601652


##########
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:
   Thanks for your review.
   I think this way may still have the same problem.
   
   Assume that there are two threads holding the same StoreFile object.
   Let's take a look at this situation:
   ```java
   // Time1 RSMobFileCleanerChore (Thread A) executing this code, and reader is 
null
   // now needCreateReader is true
   boolean needCreateReader = sf.getReader() == null; 
   
   // Time2 another thread (Thread B) create the reader object
   
   // Time3 Thread A continue to execute the code
   // now reader has been created by Thread B, 
   // so RSMobFileCleanerChore will not create the reader object
   sf.initReader();
   byte[] mobRefData = sf.getMetadataValue(HStoreFile.MOB_FILE_REFS);
   byte[] bulkloadMarkerData = 
sf.getMetadataValue(HStoreFile.BULKLOAD_TASK_KEY);
   
   // Time4 RSMobFileCleanerChore continue to execute the code
   // because needCreateReader is true, so it will close the reader which isn't 
created by itself
   if(needCreateReader) {
     sf.closeStoreFile(true);
   }
   ```



-- 
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]

Reply via email to