From 6246aa29e7a4986c7d286dbc92896541f5329e17 Mon Sep 17 00:00:00 2001 From: stack Date: Mon, 22 Jul 2019 20:43:48 -0700 Subject: [PATCH] HBASE-22723 Have CatalogJanitor report holes and overlaps; i.e. problems it sees when doing its regular scan of hbase:meta WIP. Refactor of CatalogJanitor so it generates a Report on the state of hbase:meta when it runs. Also refactor so CJ runs even if RIT (previous it would punt on running if RIT) so it can generate a 'Report' on the interval regardless (If RIT, it just doesn't go on to do the merge/split GC as it used to). If report finds an issue, dump as a WARN message. Also make the Report available for the HMaster to pull when it goes to draw the hbck UI page (could also consider shipping the Report as part of ClusterMetrics?) Adds new, fatter Visitor to CJ, one that generates Report on each run keeping around more findings as it runs. Moved some methods around so class reads better; previous methods were randomly ordered in the class. TODO: Tests and hooking up UI and Report. --- .../hadoop/hbase/master/CatalogJanitor.java | 505 ++++++++++++------ .../hbase/master/MasterRpcServices.java | 3 +- .../hbase/master/TestCatalogJanitor.java | 26 +- 3 files changed, 364 insertions(+), 170 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java index 73fabf8ca7..cd73edd347 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java @@ -1,4 +1,4 @@ -/** +/* * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -18,23 +18,30 @@ */ package org.apache.hadoop.hbase.master; -import java.io.FileNotFoundException; +import java.io.BufferedOutputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; +import java.io.PrintStream; +import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ScheduledChore; +import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; -import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -49,47 +56,56 @@ import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.PairOfSameType; import org.apache.hadoop.hbase.util.Threads; -import org.apache.hadoop.hbase.util.Triple; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * A janitor for the catalog tables. Scans the hbase:meta catalog - * table on a period looking for unused regions to garbage collect. + * A janitor for the catalog tables. Scans the hbase:meta catalog + * table on a period. Makes a report on state of hbase:meta. Looks for unused + * regions to garbage collect. Scan of hbase:meta runs if we are NOT in maintenance + * mode, if we are NOT shutting down, AND if the assignmentmanager is loaded. + * Playing it safe, we will garbage collect no-longer needed region references + * only if there are no regions-in-transition (RIT). */ +// TODO: Only works with single hbase:meta region currently. Fix. +// TODO: Should it start over every time? Could it continue if runs into problem? @InterfaceAudience.Private public class CatalogJanitor extends ScheduledChore { private static final Logger LOG = LoggerFactory.getLogger(CatalogJanitor.class.getName()); - private final AtomicBoolean alreadyRunning = new AtomicBoolean(false); private final AtomicBoolean enabled = new AtomicBoolean(true); private final MasterServices services; - private final Connection connection; - // PID of the last Procedure launched herein. Keep around for Tests. + /** + * Saved report. Result of last successful run of Scan. May be stale. Check its date. + * One is produced on each scan done by the CJ. + */ + private volatile Report report; CatalogJanitor(final MasterServices services) { super("CatalogJanitor-" + services.getServerName().toShortString(), services, services.getConfiguration().getInt("hbase.catalogjanitor.interval", 300000)); this.services = services; - this.connection = services.getConnection(); } @Override protected boolean initialChore() { try { - if (this.enabled.get()) scan(); + if (getEnabled()) { + this.report = scan(); + if (!this.report.isEmpty()) { + LOG.warn(toString()); + } + } } catch (IOException e) { - LOG.warn("Failed initial scan of catalog table", e); + LOG.warn("Failed initial janitorial scan of hbase:meta table", e); return false; } return true; } - /** - * @param enabled - */ - public boolean setEnabled(final boolean enabled) { + boolean setEnabled(final boolean enabled) { boolean alreadyEnabled = this.enabled.getAndSet(enabled); // If disabling is requested on an already enabled chore, we could have an active // scan still going on, callers might not be aware of that and do further action thinkng @@ -103,6 +119,7 @@ public class CatalogJanitor extends ScheduledChore { return alreadyEnabled; } + boolean getEnabled() { return this.enabled.get(); } @@ -111,142 +128,52 @@ public class CatalogJanitor extends ScheduledChore { protected void chore() { try { AssignmentManager am = this.services.getAssignmentManager(); - if (this.enabled.get() && !this.services.isInMaintenanceMode() && - !this.services.getServerManager().isClusterShutdown() && am != null && - am.isMetaLoaded() && !am.hasRegionsInTransition()) { - scan(); + if (getEnabled() && !this.services.isInMaintenanceMode() && + !this.services.getServerManager().isClusterShutdown() && + isMetaLoaded(am)) { + this.report = scan(); + if (!this.report.isEmpty()) { + LOG.warn(toString()); + } } else { - LOG.warn("CatalogJanitor is disabled! Enabled=" + this.enabled.get() + - ", maintenanceMode=" + this.services.isInMaintenanceMode() + ", am=" + am + - ", metaLoaded=" + (am != null && am.isMetaLoaded()) + ", hasRIT=" + - (am != null && am.hasRegionsInTransition()) + " clusterShutDown=" + this.services - .getServerManager().isClusterShutdown()); + LOG.warn("CatalogJanitor is disabled! enabled=" + getEnabled() + + ", maintenanceMode=" + this.services.isInMaintenanceMode() + + ", am=" + am + + ", metaLoaded=" + isMetaLoaded(am) + + ", hasRIT=" + isRIT(am) + + " clusterShutDown=" + this.services.getServerManager().isClusterShutdown()); } } catch (IOException e) { - LOG.warn("Failed scan of catalog table", e); + LOG.warn("Failed janitorial scan of hbase:meta table", e); } } - /** - * Scans hbase:meta and returns a number of scanned rows, and a map of merged - * regions, and an ordered map of split parents. - * @return triple of scanned rows, map of merged regions and map of split - * parent regioninfos - * @throws IOException - */ - Triple, Map> - getMergedRegionsAndSplitParents() throws IOException { - return getMergedRegionsAndSplitParents(null); - } - - /** - * Scans hbase:meta and returns a number of scanned rows, and a map of merged - * regions, and an ordered map of split parents. if the given table name is - * null, return merged regions and split parents of all tables, else only the - * specified table - * @param tableName null represents all tables - * @return triple of scanned rows, and map of merged regions, and map of split - * parent regioninfos - * @throws IOException - */ - Triple, Map> - getMergedRegionsAndSplitParents(final TableName tableName) throws IOException { - final boolean isTableSpecified = (tableName != null); - // TODO: Only works with single hbase:meta region currently. Fix. - final AtomicInteger count = new AtomicInteger(0); - // Keep Map of found split parents. There are candidates for cleanup. - // Use a comparator that has split parents come before its daughters. - final Map splitParents = new TreeMap<>(new SplitParentFirstComparator()); - final Map mergedRegions = new TreeMap<>(RegionInfo.COMPARATOR); - // This visitor collects split parents and counts rows in the hbase:meta table - - MetaTableAccessor.Visitor visitor = new MetaTableAccessor.Visitor() { - @Override - public boolean visit(Result r) throws IOException { - if (r == null || r.isEmpty()) return true; - count.incrementAndGet(); - RegionInfo info = MetaTableAccessor.getRegionInfo(r); - if (info == null) return true; // Keep scanning - if (isTableSpecified - && info.getTable().compareTo(tableName) > 0) { - // Another table, stop scanning - return false; - } - if (LOG.isTraceEnabled()) LOG.trace("" + info + " IS-SPLIT_PARENT=" + info.isSplitParent()); - if (info.isSplitParent()) splitParents.put(info, r); - if (r.getValue(HConstants.CATALOG_FAMILY, HConstants.MERGEA_QUALIFIER) != null) { - mergedRegions.put(info, r); - } - // Returning true means "keep scanning" - return true; - } - }; - - // Run full scan of hbase:meta catalog table passing in our custom visitor with - // the start row - MetaTableAccessor.scanMetaForTableRegions(this.connection, visitor, tableName); - - return new Triple<>(count.get(), mergedRegions, splitParents); + private static boolean isMetaLoaded(AssignmentManager am) { + return am != null && am.isMetaLoaded(); } - /** - * If merged region no longer holds reference to the merge regions, archive - * merge region on hdfs and perform deleting references in hbase:meta - * @param mergedRegion - * @return true if we delete references in merged region on hbase:meta and archive - * the files on the file system - * @throws IOException - */ - boolean cleanMergeRegion(final RegionInfo mergedRegion, - final RegionInfo regionA, final RegionInfo regionB) throws IOException { - FileSystem fs = this.services.getMasterFileSystem().getFileSystem(); - Path rootdir = this.services.getMasterFileSystem().getRootDir(); - Path tabledir = FSUtils.getTableDir(rootdir, mergedRegion.getTable()); - TableDescriptor htd = getTableDescriptor(mergedRegion.getTable()); - HRegionFileSystem regionFs = null; - try { - regionFs = HRegionFileSystem.openRegionFromFileSystem( - this.services.getConfiguration(), fs, tabledir, mergedRegion, true); - } catch (IOException e) { - LOG.warn("Merged region does not exist: " + mergedRegion.getEncodedName()); - } - if (regionFs == null || !regionFs.hasReferences(htd)) { - LOG.debug("Deleting region " + regionA.getShortNameToLog() + " and " - + regionB.getShortNameToLog() - + " from fs because merged region no longer holds references"); - ProcedureExecutor pe = this.services.getMasterProcedureExecutor(); - pe.submitProcedure(new GCMergedRegionsProcedure(pe.getEnvironment(), - mergedRegion, regionA, regionB)); - // Remove from in-memory states - this.services.getAssignmentManager().getRegionStates().deleteRegion(regionA); - this.services.getAssignmentManager().getRegionStates().deleteRegion(regionB); - this.services.getServerManager().removeRegion(regionA); - this.services.getServerManager().removeRegion(regionB); - return true; - } - return false; + private static boolean isRIT(AssignmentManager am) { + return isMetaLoaded(am) && am.hasRegionsInTransition(); } /** * Run janitorial scan of catalog hbase:meta table looking for * garbage to collect. - * @return number of archiving jobs started. - * @throws IOException */ - int scan() throws IOException { - int result = 0; - + Report scan() throws IOException { + Report report = null; try { if (!alreadyRunning.compareAndSet(false, true)) { LOG.debug("CatalogJanitor already running"); - return result; + return report; + } + report = visit(); + if (isRIT(this.services.getAssignmentManager())) { + LOG.warn("Playing-it-safe skipping merge/split gc'ing of regions from hbase:meta while " + + "regions-in-transition (RIT)"); } - Triple, Map> scanTriple = - getMergedRegionsAndSplitParents(); - /** - * clean merge regions first - */ - Map mergedRegions = scanTriple.getSecond(); + // Clean merge regions first + Map mergedRegions = report.mergedRegions; for (Map.Entry e : mergedRegions.entrySet()) { if (this.services.isInMaintenanceMode()) { // Stop cleaning if the master is in maintenance mode @@ -264,14 +191,12 @@ public class CatalogJanitor extends ScheduledChore { + " in merged region " + e.getKey().getShortNameToLog()); } else { if (cleanMergeRegion(e.getKey(), regionA, regionB)) { - result++; + report.gcs++; } } } - /** - * clean split parents - */ - Map splitParents = scanTriple.getThird(); + // Clean split parents + Map splitParents = report.splitParents; // Now work on our list of found parents. See if any we can clean up. // regions whose parents are still around @@ -284,7 +209,7 @@ public class CatalogJanitor extends ScheduledChore { if (!parentNotCleaned.contains(e.getKey().getEncodedName()) && cleanParent(e.getKey(), e.getValue())) { - result++; + report.gcs++; } else { // We could not clean the parent, so it's daughters should not be // cleaned either (HBASE-6160) @@ -294,15 +219,58 @@ public class CatalogJanitor extends ScheduledChore { parentNotCleaned.add(daughters.getSecond().getEncodedName()); } } - return result; + return report; } finally { alreadyRunning.set(false); } } + Report visit() throws IOException { + MetaTableVisitor visitor = new MetaTableVisitor(this.services); + // Null tablename means scan all of meta. + MetaTableAccessor.scanMetaForTableRegions(this.services.getConnection(), visitor, null); + return visitor.getReport(); + } + + /** - * Compare HRegionInfos in a way that has split parents sort BEFORE their - * daughters. + * If merged region no longer holds reference to the merge regions, archive + * merge region on hdfs and perform deleting references in hbase:meta + * @return true if we delete references in merged region on hbase:meta and archive + * the files on the file system + */ + private boolean cleanMergeRegion(final RegionInfo mergedRegion, + final RegionInfo regionA, final RegionInfo regionB) throws IOException { + FileSystem fs = this.services.getMasterFileSystem().getFileSystem(); + Path rootdir = this.services.getMasterFileSystem().getRootDir(); + Path tabledir = FSUtils.getTableDir(rootdir, mergedRegion.getTable()); + TableDescriptor htd = getTableDescriptor(mergedRegion.getTable()); + HRegionFileSystem regionFs = null; + try { + regionFs = HRegionFileSystem.openRegionFromFileSystem( + this.services.getConfiguration(), fs, tabledir, mergedRegion, true); + } catch (IOException e) { + LOG.warn("Merged region does not exist: " + mergedRegion.getEncodedName()); + } + if (regionFs == null || !regionFs.hasReferences(htd)) { + LOG.debug("Deleting region " + regionA.getShortNameToLog() + " and " + + regionB.getShortNameToLog() + + " from fs because merged region no longer holds references"); + ProcedureExecutor pe = this.services.getMasterProcedureExecutor(); + pe.submitProcedure(new GCMergedRegionsProcedure(pe.getEnvironment(), + mergedRegion, regionA, regionB)); + // Remove from in-memory states + this.services.getAssignmentManager().getRegionStates().deleteRegion(regionA); + this.services.getAssignmentManager().getRegionStates().deleteRegion(regionB); + this.services.getServerManager().removeRegion(regionA); + this.services.getServerManager().removeRegion(regionB); + return true; + } + return false; + } + + /** + * Compare HRegionInfos in a way that has split parents sort BEFORE their daughters. */ static class SplitParentFirstComparator implements Comparator { Comparator rowEndKeyComparator = new Bytes.RowEndKeyComparator(); @@ -310,14 +278,22 @@ public class CatalogJanitor extends ScheduledChore { public int compare(RegionInfo left, RegionInfo right) { // This comparator differs from the one RegionInfo in that it sorts // parent before daughters. - if (left == null) return -1; - if (right == null) return 1; + if (left == null) { + return -1; + } + if (right == null) { + return 1; + } // Same table name. int result = left.getTable().compareTo(right.getTable()); - if (result != 0) return result; + if (result != 0) { + return result; + } // Compare start keys. result = Bytes.compareTo(left.getStartKey(), right.getStartKey()); - if (result != 0) return result; + if (result != 0) { + return result; + } // Compare end keys, but flip the operands so parent comes first result = rowEndKeyComparator.compare(right.getEndKey(), left.getEndKey()); @@ -332,7 +308,6 @@ public class CatalogJanitor extends ScheduledChore { * metaRegionName * @return True if we removed parent from meta table and from * the filesystem. - * @throws IOException */ boolean cleanParent(final RegionInfo parent, Result rowContent) throws IOException { @@ -381,11 +356,11 @@ public class CatalogJanitor extends ScheduledChore { * @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. - * @throws IOException + * region directory exists in the filesystem and then the second boolean says + * whether the daughter has references to the parent. */ - Pair checkDaughterInFs(final RegionInfo parent, final RegionInfo daughter) + private Pair checkDaughterInFs(final RegionInfo parent, + final RegionInfo daughter) throws IOException { if (daughter == null) { return new Pair<>(Boolean.FALSE, Boolean.FALSE); @@ -397,7 +372,7 @@ public class CatalogJanitor extends ScheduledChore { Path daughterRegionDir = new Path(tabledir, daughter.getEncodedName()); - HRegionFileSystem regionFs = null; + HRegionFileSystem regionFs; try { if (!FSUtils.isExists(fs, daughterRegionDir)) { @@ -425,23 +400,19 @@ public class CatalogJanitor extends ScheduledChore { + ", to: " + parent.getEncodedName() + " assuming has references", e); return new Pair<>(Boolean.TRUE, Boolean.TRUE); } - return new Pair<>(Boolean.TRUE, Boolean.valueOf(references)); + return new Pair<>(Boolean.TRUE, references); } - private TableDescriptor getTableDescriptor(final TableName tableName) - throws FileNotFoundException, IOException { + private TableDescriptor getTableDescriptor(final TableName tableName) throws IOException { return this.services.getTableDescriptors().get(tableName); } /** * Checks if the specified region has merge qualifiers, if so, try to clean * them - * @param region * @return true if the specified region doesn't have merge qualifier now - * @throws IOException */ - public boolean cleanMergeQualifier(final RegionInfo region) - throws IOException { + public boolean cleanMergeQualifier(final RegionInfo region) throws IOException { // Get merge regions if it is a merged region and already has merge // qualifier Pair mergeRegions = MetaTableAccessor @@ -458,7 +429,215 @@ public class CatalogJanitor extends ScheduledChore { + " has only one merge qualifier in META."); return false; } - return cleanMergeRegion(region, mergeRegions.getFirst(), - mergeRegions.getSecond()); + return cleanMergeRegion(region, mergeRegions.getFirst(), mergeRegions.getSecond()); + } + + /** + * Catalog Janitor Scan report. + * An instance is created per run of the CatalogJanitor. + */ + static class Report { + /** + * Note time of Report creation + */ + final long now = System.currentTimeMillis(); + + final Map splitParents = new TreeMap<>(new SplitParentFirstComparator()); + final Map mergedRegions = new TreeMap<>(RegionInfo.COMPARATOR); + final Map holes = new TreeMap<>(RegionInfo.COMPARATOR); + final Map> overlaps = new TreeMap<>(Bytes.BYTES_COMPARATOR); + final Map deadServers = new HashMap(); + final Map unknownServers = new HashMap(); + final List emptyRegionInfo = new ArrayList<>(); + int count = 0; + int gcs = 0; + + @VisibleForTesting + Report () {} + + @Override + public String toString() { + if (isEmpty()) { + return null; + } + try (ByteArrayOutputStream os = new ByteArrayOutputStream()) { + try (PrintStream ps = new PrintStream(os)) { + print(ps); + } + return Bytes.toString(os.toByteArray()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + + /** + * @return True if an 'empty' report -- no problems found. + */ + boolean isEmpty() { + return this.holes.isEmpty() && this.overlaps.isEmpty() && this.unknownServers.isEmpty() && + this.emptyRegionInfo.isEmpty(); + + } + + void print(PrintStream out) { + for (Map.Entry e: this.holes.entrySet()) { + out.println("Hole " + e.getKey() + ", " + e.getValue()); + } + for (Map.Entry> e: this.overlaps.entrySet()) { + out.println(e.getValue().size() + " overlaps centered on " + Bytes.toString(e.getKey()) + + "; " + e.getValue().toString()); + } + for (byte [] k: this.emptyRegionInfo) { + out.println("Empty info:regioninfo " + Bytes.toString(k)); + } + for (Map.Entry e: this.unknownServers.entrySet()) { + out.println("Unknown " + e.getKey() + " for " + e.getValue()); + } + } + } + + /** + * Visitor we use in here in CatalogJanitor to go against hbase:meta table. + * This visitor collects split parents and counts rows in the hbase:meta table. + * Also runs hbase:meta consistency checks. Generates a {@link Report} + */ + private static class MetaTableVisitor implements MetaTableAccessor.Visitor { + // Keep Map of found split parents. These are candidates for cleanup. + // Use a comparator that has split parents come before its daughters. + + final MasterServices services; + private final Report report = new Report(); + + /** + * RegionInfo from previous row. + */ + private RegionInfo previous; + + MetaTableVisitor(MasterServices services) { + this.services = services; + } + + Report getReport() { + return this.report; + } + + @Override + public boolean visit(Result r) { + if (r == null || r.isEmpty()) { + return true; + } + this.report.count++; + RegionInfo regionInfo = metaTableConsistencyCheck(r); + if (regionInfo == null) { + this.report.emptyRegionInfo.add(r.getRow()); + return true; // Keep scanning + } + + LOG.trace(regionInfo.toString()); + if (regionInfo.isSplitParent()) { // splitParent means split and offline. + this.report.splitParents.put(regionInfo, r); + } + if (r.getValue(HConstants.CATALOG_FAMILY, HConstants.MERGEA_QUALIFIER) != null) { + this.report.mergedRegions.put(regionInfo, r); + } + // Returning true means "keep scanning" + return true; + } + + /** + * Check row. + * @param metaTableRow Row from hbase:meta table. + * @return Returns default regioninfo found in row parse as a convenience to save + * on having to do a double-parse of Result. + */ + private RegionInfo metaTableConsistencyCheck(Result metaTableRow) { + RegionLocations locations = MetaTableAccessor.getRegionLocations(metaTableRow); + if (locations == null) { + return null; + } + checkServer(locations); + RegionInfo ri = locations.getDefaultRegionLocation().getRegion(); + if (this.previous != null) { + if (isTableTransition(ri)) { + if (!isEnd(this.previous) || !isStart(ri)) { + this.report.holes.put(this.previous, ri); + } + } else { + // Inside a table. + if (!isAdjacent(this.previous, ri)) { + this.report.holes.put(this.previous, ri); + } + // Check for overlaps. + if (Bytes.equals(this.previous.getStartKey(), ri.getStartKey())) { + addOverlap(this.previous.getStartKey(), ri); + } else if (Bytes.equals(this.previous.getEndKey(), ri.getEndKey())) { + addOverlap(this.previous.getEndKey(), ri); + } + } + } + this.previous = ri; + return ri; + } + + /** + * Run through referenced servers and save off unknown and the dead. + */ + private void checkServer(RegionLocations locations) { + if (locations == null) { + return; + } + // Check referenced servers are known/online. + for (HRegionLocation location: locations.getRegionLocations()) { + ServerName sn = location.getServerName(); + ServerManager.ServerLiveState state = this.services.getServerManager(). + isServerKnownAndOnline(sn); + switch (state) { + case DEAD: + this.report.deadServers.put(sn, location.getRegion()); + break; + + case UNKNOWN: + this.report.unknownServers.put(sn, location.getRegion()); + break; + + default: + break; + } + } + } + + private void addOverlap(byte [] key, RegionInfo ri) { + this.report.overlaps.computeIfAbsent(key, k -> new ArrayList<>()).add(ri); + } + + /** + * @return True iff first row in hbase:meta or if we've broached a new table in hbase:meta + */ + private boolean isTableTransition(RegionInfo ri) { + return this.previous == null || !this.previous.getTable().equals(ri.getTable()); + } + } + + + /** + * @return True if this is first Region in table + */ + private static boolean isStart(RegionInfo ri) { + return Bytes.equals(ri.getStartKey(), HConstants.EMPTY_START_ROW); + } + + /** + * @return True if this is last Region in table + */ + private static boolean isEnd(RegionInfo ri) { + return Bytes.equals(ri.getEndKey(), HConstants.EMPTY_START_ROW); + } + + /** + * @return True if regions are adjacent + */ + private static boolean isAdjacent(RegionInfo before, RegionInfo after) { + return Bytes.equals(before.getEndKey(), after.getStartKey()); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 8188cf2c11..146ca64b41 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -1489,7 +1489,8 @@ public class MasterRpcServices extends RSRpcServices RunCatalogScanRequest req) throws ServiceException { rpcPreCheck("runCatalogScan"); try { - return ResponseConverter.buildRunCatalogScanResponse(master.catalogJanitorChore.scan()); + CatalogJanitor.Report report = this.master.catalogJanitorChore.scan(); + return ResponseConverter.buildRunCatalogScanResponse(report.gcs); } catch (IOException ioe) { throw new ServiceException(ioe); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java index 27eec65274..e810bfbae6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -59,7 +59,6 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.HFileArchiveUtil; -import org.apache.hadoop.hbase.util.Triple; import org.apache.zookeeper.KeeperException; import org.junit.After; import org.junit.Before; @@ -107,6 +106,14 @@ public class TestCatalogJanitor { this.masterServices.stop("DONE"); } + @Test + public void testGetReport() throws IOException { + /* + this.janitor.scan(); + CatalogJanitor.Report report = this.janitor.getReport(); + */ + } + /** * Test clearing a split parent. */ @@ -309,8 +316,13 @@ public class TestCatalogJanitor { final Map mergedRegions = new TreeMap<>(); CatalogJanitor spy = spy(this.janitor); - doReturn(new Triple<>(10, mergedRegions, splitParents)).when(spy). - getMergedRegionsAndSplitParents(); + + CatalogJanitor.Report report = new CatalogJanitor.Report(); + report.count = 10; + report.mergedRegions.putAll(mergedRegions); + report.splitParents.putAll(splitParents); + + doReturn(report).when(spy).visit(); // Create ref from splita to parent LOG.info("parent=" + parent.getShortNameToLog() + ", splita=" + splita.getShortNameToLog()); @@ -319,14 +331,16 @@ public class TestCatalogJanitor { LOG.info("Created reference " + splitaRef); // Parent and splita should not be removed because a reference from splita to parent. - assertEquals(0, spy.scan()); + report = spy.scan(); + assertEquals(0, report.gcs); // Now delete the ref FileSystem fs = FileSystem.get(HTU.getConfiguration()); assertTrue(fs.delete(splitaRef, true)); //now, both parent, and splita can be deleted - assertEquals(2, spy.scan()); + report = spy.scan(); + assertEquals(2, report.gcs); } /** -- 2.19.1