[GitHub] [hbase] virajjasani commented on a diff in pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion

[GitBox <gi...@apache.org>]

virajjasani commented on code in PR #4986:
URL: https://github.com/apache/hbase/pull/4986#discussion_r1081618087


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java:
##########
@@ -387,59 +390,45 @@ private static boolean hasNoReferences(final Pair<Boolean, Boolean> p) {
   }
 
   /**
-   * Checks if a daughter region -- either splitA or splitB -- still holds references to parent.
-   * @param parent   Parent region
-   * @param daughter Daughter region
-   * @return A pair where the first boolean says whether or not the daughter region directory exists
-   *         in the filesystem and then the second boolean says whether the daughter has references
-   *         to the parent.
+   * Checks if a region still holds references to parent.
+   * @param tableName The table for the region
+   * @param region    The region to check
+   * @return A pair where the first boolean says whether the region directory exists in the
+   *         filesystem and then the second boolean says whether the region has references to a
+   *         parent.
    */
-  private static Pair<Boolean, Boolean> checkDaughterInFs(MasterServices services,
-    final RegionInfo parent, final RegionInfo daughter) throws IOException {
-    if (daughter == null) {
+  private static Pair<Boolean, Boolean> checkRegionReferences(MasterServices services,
+    TableName tableName, RegionInfo region) throws IOException {
+    if (region == null) {
       return new Pair<>(Boolean.FALSE, Boolean.FALSE);
     }
 
     FileSystem fs = services.getMasterFileSystem().getFileSystem();
     Path rootdir = services.getMasterFileSystem().getRootDir();
-    Path tabledir = CommonFSUtils.getTableDir(rootdir, daughter.getTable());
-
-    Path daughterRegionDir = new Path(tabledir, daughter.getEncodedName());
-
-    HRegionFileSystem regionFs;
+    Path tabledir = CommonFSUtils.getTableDir(rootdir, tableName);
+    Path regionDir = new Path(tabledir, region.getEncodedName());
 
     try {
-      if (!CommonFSUtils.isExists(fs, daughterRegionDir)) {
+      if (!CommonFSUtils.isExists(fs, regionDir)) {
         return new Pair<>(Boolean.FALSE, Boolean.FALSE);
       }
     } catch (IOException ioe) {
-      LOG.error("Error trying to determine if daughter region exists, "
-        + "assuming exists and has references", ioe);
+      LOG.error("Error trying to determine if region exists, assuming exists and has references",
+        ioe);
       return new Pair<>(Boolean.TRUE, Boolean.TRUE);

Review Comment:
   So this is the big difference b/ the two implementations right? 



-- 
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-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org