wchevreuil commented on a change in pull request #45: HBASE-23371 [HBCK2] 
Provide client side method for removing ghost regions in meta.
URL: 
https://github.com/apache/hbase-operator-tools/pull/45#discussion_r356684091
 
 

 ##########
 File path: 
hbase-hbck2/src/main/java/org/apache/hbase/FsRegionsMetaRecoverer.java
 ##########
 @@ -70,61 +77,207 @@ public FsRegionsMetaRecoverer(Configuration 
configuration) throws IOException {
     this.fs = fileSystem;
   }
 
-  private List<Path> getTableRegionsDirs(String table) throws Exception {
+  private List<Path> getTableRegionsDirs(String table) throws IOException {
     String hbaseRoot = this.config.get(HConstants.HBASE_DIR);
     Path tableDir = FSUtils.getTableDir(new Path(hbaseRoot), 
TableName.valueOf(table));
     return FSUtils.getRegionDirs(fs, tableDir);
   }
 
   public Map<TableName,List<Path>> reportTablesMissingRegions(final 
List<String> namespacesOrTables)
       throws IOException {
-    final Map<TableName,List<Path>> result = new HashMap<>();
-    List<TableName> tableNames = 
MetaTableAccessor.getTableStates(this.conn).keySet().stream()
-      .filter(tableName -> {
-        if(namespacesOrTables==null || namespacesOrTables.isEmpty()){
-          return true;
-        } else {
-          Optional<String> findings = namespacesOrTables.stream().filter(
-            name -> (name.indexOf(":") > 0) ?
-              tableName.equals(TableName.valueOf(name)) :
-              tableName.getNamespaceAsString().equals(name)).findFirst();
-          return findings.isPresent();
+    InternalMetaChecker<Path> missingChecker = new InternalMetaChecker<>();
+    return missingChecker.reportTablesRegions(namespacesOrTables, 
this::findMissingRegionsInMETA);
+  }
+
+  public Map<TableName,List<RegionInfo>>
+      reportTablesExtraRegions(final List<String> namespacesOrTables) throws 
IOException {
+    InternalMetaChecker<RegionInfo> extraChecker = new InternalMetaChecker<>();
+    return extraChecker.reportTablesRegions(namespacesOrTables, 
this::findExtraRegionsInMETA);
+  }
+
+  List<Path> findMissingRegionsInMETA(String table) throws IOException {
+    InternalMetaChecker<Path> missingChecker = new InternalMetaChecker<>();
+    return missingChecker.checkRegionsInMETA(table, (regions, dirs) -> {
+      final List<Path> missingRegions = new ArrayList<>();
+      HashSet<String> regionsInMeta = regions.stream().map(info ->
+        info.getEncodedName()).collect(Collectors.toCollection(HashSet::new));
+      dirs.forEach(dir -> {
+        if(!regionsInMeta.contains(dir.getName())){
+          LOG.debug("{} is not in META.", dir);
+          missingRegions.add(dir);
         }
-      }).collect(Collectors.toList());
-    tableNames.stream().forEach(tableName -> {
-      try {
-        result.put(tableName,
-          
findMissingRegionsInMETA(tableName.getNameWithNamespaceInclAsString()));
-      } catch (Exception e) {
-        LOG.warn("Can't get missing regions from meta", e);
-      }
+      });
+      return missingRegions;
+    });
+  }
+
+  List<RegionInfo> findExtraRegionsInMETA(String table) throws IOException {
+    InternalMetaChecker<RegionInfo> extraChecker = new InternalMetaChecker<>();
+    return extraChecker.checkRegionsInMETA(table, (regions,dirs) -> {
+      final List<RegionInfo> extraRegions = new ArrayList<>();
+      HashSet<String> regionsInHDFS = dirs.stream().map(dir -> dir.getName())
+        .collect(Collectors.toCollection(HashSet::new));
+      regions.forEach(region -> {
+        if(!regionsInHDFS.contains(region.getEncodedName())) {
+          LOG.debug("Region {} found in META, but not in HDFS.", 
region.getEncodedName());
+          extraRegions.add(region);
+        }
+      });
+      return extraRegions;
     });
-    return result;
-  }
-
-  List<Path> findMissingRegionsInMETA(String table) throws Exception {
-    final List<Path> missingRegions = new ArrayList<>();
-    final List<Path> regionsDirs = getTableRegionsDirs(table);
-    TableName tableName = TableName.valueOf(table);
-    List<RegionInfo> regionInfos = MetaTableAccessor.
-      getTableRegions(this.conn, tableName, false);
-    HashSet<String> regionsInMeta = regionInfos.stream().map(info ->
-      info.getEncodedName()).collect(Collectors.toCollection(HashSet::new));
-    for(final Path regionDir : regionsDirs){
-      if (!regionsInMeta.contains(regionDir.getName())) {
-        LOG.debug(regionDir + "is not in META.");
-        missingRegions.add(regionDir);
-      }
-    }
-    return missingRegions;
   }
 
-  public void putRegionInfoFromHdfsInMeta(Path region) throws IOException {
+  void putRegionInfoFromHdfsInMeta(Path region) throws IOException {
     RegionInfo info = HRegionFileSystem.loadRegionInfoFileContent(fs, region);
     MetaTableAccessor.addRegionToMeta(conn, info);
   }
 
-  @Override public void close() throws IOException {
+  List<String> addMissingRegionsInMeta(List<Path> regionsPath) throws 
IOException {
+    List<String> reAddedRegionsEncodedNames = new ArrayList<>();
+    for(Path regionPath : regionsPath){
+      this.putRegionInfoFromHdfsInMeta(regionPath);
+      reAddedRegionsEncodedNames.add(regionPath.getName());
+    }
+    return reAddedRegionsEncodedNames;
+  }
+
+  public Pair<List<String>, List<ExecutionException>> 
addMissingRegionsInMetaForTables(
+      List<String> nameSpaceOrTable) throws IOException {
+    InternalMetaChecker<Path> missingChecker = new InternalMetaChecker<>();
+    return 
missingChecker.processRegionsMetaCleanup(this::reportTablesMissingRegions,
+      this::addMissingRegionsInMeta, nameSpaceOrTable);
+  }
+
+  public Pair<List<String>, List<ExecutionException>> 
removeExtraRegionsFromMetaForTables(
+    List<String> nameSpaceOrTable) throws IOException {
+    if(nameSpaceOrTable.size()>0) {
+      InternalMetaChecker<RegionInfo> extraChecker = new 
InternalMetaChecker<>();
+      return 
extraChecker.processRegionsMetaCleanup(this::reportTablesExtraRegions, regions 
-> {
+        MetaTableAccessor.deleteRegionInfos(conn, regions);
+        return regions.stream().map(r -> 
r.getEncodedName()).collect(Collectors.toList());
+      }, nameSpaceOrTable);
+    } else {
+      return null;
+    }
+  }
+
+
+  @Override
+  public void close() throws IOException {
     this.conn.close();
   }
+
+  private class InternalMetaChecker<T> {
+
+    List<T> checkRegionsInMETA(String table,
+        CheckingFunction<List<RegionInfo>, List<Path>, T> checkingFunction) 
throws IOException {
+      final List<Path> regionsDirs = getTableRegionsDirs(table);
+      TableName tableName = TableName.valueOf(table);
+      List<RegionInfo> regions = MetaTableAccessor.
+        getTableRegions(FsRegionsMetaRecoverer.this.conn, tableName, false);
+      return checkingFunction.check(regions, regionsDirs);
+    }
+
+    Map<TableName,List<T>> reportTablesRegions(final List<String> 
namespacesOrTables,
+      ExecFunction<List<T>, String> checkingFunction) throws IOException {
+      final Map<TableName,List<T>> result = new HashMap<>();
+      List<TableName> tableNames = MetaTableAccessor.
+        getTableStates(FsRegionsMetaRecoverer.this.conn).keySet().stream()
+          .filter(tableName -> {
+            if(namespacesOrTables==null || namespacesOrTables.isEmpty()){
+              return true;
+            } else {
+              Optional<String> findings = namespacesOrTables.stream().filter(
 
 Review comment:
   Problem is that I'm allowing _namespacesOrTables_ value to be a namespace, 
in which case, all tables under the given namespace should be checked. If we 
make it a _TableName_ type, I guess we won't be able to pass a namespace as 
value.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to