From a3153bfde530f804046378001af78dab5a0f116a Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 10 Jun 2022 15:47:23 +0200 Subject: [PATCH] HBASE-27095 HbckChore should produce a report In #4470 for HBASE-26192, it was noted that the HbckChore is kind of a pain to use and test because it maintains a bunch of local state. By contract, the CatalogJanitorChore makes a nice self-contained report. Let's update HbckChore to do the same. Signed-off-by: Andrew Purtell --- .../apache/hadoop/hbase/master/HMaster.java | 1 + .../hbase/master/MasterRpcServices.java | 1 + .../hbase/master/{ => hbck}/HbckChore.java | 213 +++++------------- .../hadoop/hbase/master/hbck/HbckReport.java | 110 +++++++++ .../hbase/master/janitor/CatalogJanitor.java | 10 +- ...{Report.java => CatalogJanitorReport.java} | 2 +- .../hbase/master/janitor/MetaFixer.java | 6 +- .../master/janitor/ReportMakingVisitor.java | 4 +- .../resources/hbase-webapps/master/hbck.jsp | 125 +++++----- .../apache/hadoop/hbase/client/TestHbck.java | 20 +- .../master/TestMasterChoreScheduled.java | 1 + .../master/assignment/TestHbckChore.java | 48 ++-- .../master/janitor/TestCatalogJanitor.java | 2 +- .../janitor/TestCatalogJanitorCluster.java | 8 +- .../hbase/master/janitor/TestMetaFixer.java | 32 +-- 15 files changed, 304 insertions(+), 279 deletions(-) rename hbase-server/src/main/java/org/apache/hadoop/hbase/master/{ => hbck}/HbckChore.java (56%) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckReport.java rename hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/{Report.java => CatalogJanitorReport.java} (99%) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 195a504fb84f..b5e2c1c878d7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -116,6 +116,7 @@ import org.apache.hadoop.hbase.master.cleaner.LogCleaner; import org.apache.hadoop.hbase.master.cleaner.ReplicationBarrierCleaner; import org.apache.hadoop.hbase.master.cleaner.SnapshotCleanerChore; +import org.apache.hadoop.hbase.master.hbck.HbckChore; import org.apache.hadoop.hbase.master.janitor.CatalogJanitor; import org.apache.hadoop.hbase.master.locking.LockManager; import org.apache.hadoop.hbase.master.normalizer.RegionNormalizerFactory; 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 6beb5361da76..401ed0d6b803 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 @@ -76,6 +76,7 @@ import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException; import org.apache.hadoop.hbase.ipc.ServerRpcController; import org.apache.hadoop.hbase.master.assignment.RegionStates; +import org.apache.hadoop.hbase.master.hbck.HbckChore; import org.apache.hadoop.hbase.master.janitor.MetaFixer; import org.apache.hadoop.hbase.master.locking.LockProcedure; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckChore.java similarity index 56% rename from hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChore.java rename to hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckChore.java index 41e0d3addffe..0a3d310c98c6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckChore.java @@ -15,16 +15,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.hadoop.hbase.master; +package org.apache.hadoop.hbase.master.hbck; import java.io.IOException; +import java.time.Instant; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.MetaTableAccessor; @@ -32,6 +32,8 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableState; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.HbckRegionInfo; @@ -55,51 +57,14 @@ public class HbckChore extends ScheduledChore { private final MasterServices master; /** - * This map contains the state of all hbck items. It maps from encoded region name to - * HbckRegionInfo structure. The information contained in HbckRegionInfo is used to detect and - * correct consistency (hdfs/meta/deployment) problems. + * Saved report from last time this chore ran. Check its date. */ - private final Map regionInfoMap = new HashMap<>(); - - private final Set disabledTableRegions = new HashSet<>(); - private final Set splitParentRegions = new HashSet<>(); - - /** - * The regions only opened on RegionServers, but no region info in meta. - */ - private final Map orphanRegionsOnRS = new HashMap<>(); - /** - * The regions have directory on FileSystem, but no region info in meta. - */ - private final Map orphanRegionsOnFS = new HashMap<>(); - /** - * The inconsistent regions. There are three case: case 1. Master thought this region opened, but - * no regionserver reported it. case 2. Master thought this region opened on Server1, but - * regionserver reported Server2 case 3. More than one regionservers reported opened this region - */ - private final Map>> inconsistentRegions = - new HashMap<>(); - - /** - * The "snapshot" is used to save the last round's HBCK checking report. - */ - private final Map orphanRegionsOnRSSnapshot = new HashMap<>(); - private final Map orphanRegionsOnFSSnapshot = new HashMap<>(); - private final Map>> inconsistentRegionsSnapshot = - new HashMap<>(); - - /** - * The "snapshot" may be changed after checking. And this checking report "snapshot" may be - * accessed by web ui. Use this rwLock to synchronize. - */ - ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); + private volatile HbckReport lastReport = null; /** * When running, the "snapshot" may be changed when this round's checking finish. */ private volatile boolean running = false; - private volatile long checkingStartTimestamp = 0; - private volatile long checkingEndTimestamp = 0; private boolean disabled = false; @@ -115,39 +80,47 @@ public HbckChore(MasterServices master) { } } + /** + * @return Returns last published Report that comes of last successful execution of this chore. + */ + public HbckReport getLastReport() { + return lastReport; + } + @Override protected synchronized void chore() { if (isDisabled() || isRunning()) { LOG.warn("hbckChore is either disabled or is already running. Can't run the chore"); return; } - regionInfoMap.clear(); - disabledTableRegions.clear(); - splitParentRegions.clear(); - orphanRegionsOnRS.clear(); - orphanRegionsOnFS.clear(); - inconsistentRegions.clear(); - checkingStartTimestamp = EnvironmentEdgeManager.currentTime(); running = true; + final HbckReport report = new HbckReport(); + report.setCheckingStartTimestamp(Instant.ofEpochMilli(EnvironmentEdgeManager.currentTime())); try { - loadRegionsFromInMemoryState(); - loadRegionsFromRSReport(); + loadRegionsFromInMemoryState(report); + loadRegionsFromRSReport(report); try { - loadRegionsFromFS(scanForMergedParentRegions()); + loadRegionsFromFS(scanForMergedParentRegions(), report); } catch (IOException e) { LOG.warn("Failed to load the regions from filesystem", e); } - saveCheckResultToSnapshot(); } catch (Throwable t) { LOG.warn("Unexpected", t); } + report.setCheckingEndTimestamp(Instant.ofEpochMilli(EnvironmentEdgeManager.currentTime())); + this.lastReport = report; running = false; - updateAssignmentManagerMetrics(); + updateAssignmentManagerMetrics(report); } - // This function does the sanity checks of making sure the chore is not run when it is - // disabled or when it's already running. It returns whether the chore was actually run or not. - protected boolean runChore() { + /** + * Request execution of this chore's action. + * @return {@code true} if the chore was executed, {@code false} if the chore is disabled or + * already running. + */ + public boolean runChore() { + // This function does the sanity checks of making sure the chore is not run when it is + // disabled or when it's already running. It returns whether the chore was actually run or not. if (isDisabled() || isRunning()) { if (isDisabled()) { LOG.warn("hbck chore is disabled! Set " + HBCK_CHORE_INTERVAL + " > 0 to enable it."); @@ -168,25 +141,6 @@ public boolean isDisabled() { return this.disabled; } - private void saveCheckResultToSnapshot() { - // Need synchronized here, as this "snapshot" may be access by web ui. - rwLock.writeLock().lock(); - try { - orphanRegionsOnRSSnapshot.clear(); - orphanRegionsOnRS.entrySet() - .forEach(e -> orphanRegionsOnRSSnapshot.put(e.getKey(), e.getValue())); - orphanRegionsOnFSSnapshot.clear(); - orphanRegionsOnFS.entrySet() - .forEach(e -> orphanRegionsOnFSSnapshot.put(e.getKey(), e.getValue())); - inconsistentRegionsSnapshot.clear(); - inconsistentRegions.entrySet() - .forEach(e -> inconsistentRegionsSnapshot.put(e.getKey(), e.getValue())); - checkingEndTimestamp = EnvironmentEdgeManager.currentTime(); - } finally { - rwLock.writeLock().unlock(); - } - } - /** * Scan hbase:meta to get set of merged parent regions, this is a very heavy scan. * @return Return generated {@link HashSet} @@ -209,7 +163,7 @@ private HashSet scanForMergedParentRegions() throws IOException { return mergedParentRegions; } - private void loadRegionsFromInMemoryState() { + private void loadRegionsFromInMemoryState(final HbckReport report) { List regionStates = master.getAssignmentManager().getRegionStates().getRegionStates(); for (RegionState regionState : regionStates) { @@ -217,18 +171,19 @@ private void loadRegionsFromInMemoryState() { if ( master.getTableStateManager().isTableState(regionInfo.getTable(), TableState.State.DISABLED) ) { - disabledTableRegions.add(regionInfo.getRegionNameAsString()); + report.getDisabledTableRegions().add(regionInfo.getRegionNameAsString()); } // Check both state and regioninfo for split status, see HBASE-26383 if (regionState.isSplit() || regionInfo.isSplit()) { - splitParentRegions.add(regionInfo.getRegionNameAsString()); + report.getSplitParentRegions().add(regionInfo.getRegionNameAsString()); } HbckRegionInfo.MetaEntry metaEntry = new HbckRegionInfo.MetaEntry(regionInfo, regionState.getServerName(), regionState.getStamp()); - regionInfoMap.put(regionInfo.getEncodedName(), new HbckRegionInfo(metaEntry)); + report.getRegionInfoMap().put(regionInfo.getEncodedName(), new HbckRegionInfo(metaEntry)); } LOG.info("Loaded {} regions ({} disabled, {} split parents) from in-memory state", - regionStates.size(), disabledTableRegions.size(), splitParentRegions.size()); + regionStates.size(), report.getDisabledTableRegions().size(), + report.getSplitParentRegions().size()); if (LOG.isDebugEnabled()) { Map stateCountMap = new HashMap<>(); for (RegionState regionState : regionStates) { @@ -246,22 +201,23 @@ private void loadRegionsFromInMemoryState() { } if (LOG.isTraceEnabled()) { for (RegionState regionState : regionStates) { - LOG.trace("{}: {}, serverName=", regionState.getRegion(), regionState.getState(), + LOG.trace("{}: {}, serverName={}", regionState.getRegion(), regionState.getState(), regionState.getServerName()); } } } - private void loadRegionsFromRSReport() { + private void loadRegionsFromRSReport(final HbckReport report) { int numRegions = 0; Map> rsReports = master.getAssignmentManager().getRSReports(); for (Map.Entry> entry : rsReports.entrySet()) { ServerName serverName = entry.getKey(); for (byte[] regionName : entry.getValue()) { String encodedRegionName = RegionInfo.encodeRegionName(regionName); - HbckRegionInfo hri = regionInfoMap.get(encodedRegionName); + HbckRegionInfo hri = report.getRegionInfoMap().get(encodedRegionName); if (hri == null) { - orphanRegionsOnRS.put(RegionInfo.getRegionNameAsString(regionName), serverName); + report.getOrphanRegionsOnRS().put(RegionInfo.getRegionNameAsString(regionName), + serverName); continue; } hri.addServer(hri.getMetaEntry(), serverName); @@ -269,9 +225,9 @@ private void loadRegionsFromRSReport() { numRegions += entry.getValue().size(); } LOG.info("Loaded {} regions from {} regionservers' reports and found {} orphan regions", - numRegions, rsReports.size(), orphanRegionsOnRS.size()); + numRegions, rsReports.size(), report.getOrphanRegionsOnRS().size()); - for (Map.Entry entry : regionInfoMap.entrySet()) { + for (Map.Entry entry : report.getRegionInfoMap().entrySet()) { HbckRegionInfo hri = entry.getValue(); ServerName locationInMeta = hri.getMetaEntry().getRegionServer(); if (locationInMeta == null) { @@ -279,29 +235,30 @@ private void loadRegionsFromRSReport() { } if (hri.getDeployedOn().size() == 0) { // skip the offline region which belong to disabled table. - if (disabledTableRegions.contains(hri.getRegionNameAsString())) { + if (report.getDisabledTableRegions().contains(hri.getRegionNameAsString())) { continue; } // skip the split parent regions - if (splitParentRegions.contains(hri.getRegionNameAsString())) { + if (report.getSplitParentRegions().contains(hri.getRegionNameAsString())) { continue; } // Master thought this region opened, but no regionserver reported it. - inconsistentRegions.put(hri.getRegionNameAsString(), + report.getInconsistentRegions().put(hri.getRegionNameAsString(), new Pair<>(locationInMeta, new LinkedList<>())); } else if (hri.getDeployedOn().size() > 1) { // More than one regionserver reported opened this region - inconsistentRegions.put(hri.getRegionNameAsString(), + report.getInconsistentRegions().put(hri.getRegionNameAsString(), new Pair<>(locationInMeta, hri.getDeployedOn())); } else if (!hri.getDeployedOn().get(0).equals(locationInMeta)) { // Master thought this region opened on Server1, but regionserver reported Server2 - inconsistentRegions.put(hri.getRegionNameAsString(), + report.getInconsistentRegions().put(hri.getRegionNameAsString(), new Pair<>(locationInMeta, hri.getDeployedOn())); } } } - private void loadRegionsFromFS(final HashSet mergedParentRegions) throws IOException { + private void loadRegionsFromFS(final HashSet mergedParentRegions, final HbckReport report) + throws IOException { Path rootDir = master.getMasterFileSystem().getRootDir(); FileSystem fs = master.getMasterFileSystem().getFileSystem(); @@ -315,27 +272,27 @@ private void loadRegionsFromFS(final HashSet mergedParentRegions) throws LOG.warn("Failed get of encoded name from {}", regionDir); continue; } - HbckRegionInfo hri = regionInfoMap.get(encodedRegionName); + HbckRegionInfo hri = report.getRegionInfoMap().get(encodedRegionName); // If it is not in in-memory database and not a merged region, // report it as an orphan region. if (hri == null && !mergedParentRegions.contains(encodedRegionName)) { - orphanRegionsOnFS.put(encodedRegionName, regionDir); + report.getOrphanRegionsOnFS().put(encodedRegionName, regionDir); continue; } } numRegions += regionDirs.size(); } LOG.info("Loaded {} tables {} regions from filesystem and found {} orphan regions", - tableDirs.size(), numRegions, orphanRegionsOnFS.size()); + tableDirs.size(), numRegions, report.getOrphanRegionsOnFS().size()); } - private void updateAssignmentManagerMetrics() { + private void updateAssignmentManagerMetrics(final HbckReport report) { master.getAssignmentManager().getAssignmentManagerMetrics() - .updateOrphanRegionsOnRs(getOrphanRegionsOnRS().size()); + .updateOrphanRegionsOnRs(report.getOrphanRegionsOnRS().size()); master.getAssignmentManager().getAssignmentManagerMetrics() - .updateOrphanRegionsOnFs(getOrphanRegionsOnFS().size()); + .updateOrphanRegionsOnFs(report.getOrphanRegionsOnFS().size()); master.getAssignmentManager().getAssignmentManagerMetrics() - .updateInconsistentRegions(getInconsistentRegions().size()); + .updateInconsistentRegions(report.getInconsistentRegions().size()); } /** @@ -344,62 +301,4 @@ private void updateAssignmentManagerMetrics() { public boolean isRunning() { return running; } - - /** - * @return the regions only opened on RegionServers, but no region info in meta. - */ - public Map getOrphanRegionsOnRS() { - // Need synchronized here, as this "snapshot" may be changed after checking. - rwLock.readLock().lock(); - try { - return this.orphanRegionsOnRSSnapshot; - } finally { - rwLock.readLock().unlock(); - } - } - - /** - * @return the regions have directory on FileSystem, but no region info in meta. - */ - public Map getOrphanRegionsOnFS() { - // Need synchronized here, as this "snapshot" may be changed after checking. - rwLock.readLock().lock(); - try { - return this.orphanRegionsOnFSSnapshot; - } finally { - rwLock.readLock().unlock(); - } - } - - /** - * Found the inconsistent regions. There are three case: case 1. Master thought this region - * opened, but no regionserver reported it. case 2. Master thought this region opened on Server1, - * but regionserver reported Server2 case 3. More than one regionservers reported opened this - * region - * @return the map of inconsistent regions. Key is the region name. Value is a pair of location in - * meta and the regionservers which reported opened this region. - */ - public Map>> getInconsistentRegions() { - // Need synchronized here, as this "snapshot" may be changed after checking. - rwLock.readLock().lock(); - try { - return this.inconsistentRegionsSnapshot; - } finally { - rwLock.readLock().unlock(); - } - } - - /** - * Used for web ui to show when the HBCK checking started. - */ - public long getCheckingStartTimestamp() { - return this.checkingStartTimestamp; - } - - /** - * Used for web ui to show when the HBCK checking report generated. - */ - public long getCheckingEndTimestamp() { - return this.checkingEndTimestamp; - } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckReport.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckReport.java new file mode 100644 index 000000000000..6971edcd053e --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/hbck/HbckReport.java @@ -0,0 +1,110 @@ +/* + * 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 + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master.hbck; + +import java.time.Instant; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.util.HbckRegionInfo; +import org.apache.hadoop.hbase.util.Pair; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * The result of an {@link HbckChore} execution. + */ +@InterfaceAudience.Private +public class HbckReport { + + private final Map regionInfoMap = new HashMap<>(); + private final Set disabledTableRegions = new HashSet<>(); + private final Set splitParentRegions = new HashSet<>(); + private final Map orphanRegionsOnRS = new HashMap<>(); + private final Map orphanRegionsOnFS = new HashMap<>(); + private final Map>> inconsistentRegions = + new HashMap<>(); + + private Instant checkingStartTimestamp = null; + private Instant checkingEndTimestamp = null; + + /** + * Used for web ui to show when the HBCK checking started. + */ + public Instant getCheckingStartTimestamp() { + return checkingStartTimestamp; + } + + public void setCheckingStartTimestamp(Instant checkingStartTimestamp) { + this.checkingStartTimestamp = checkingStartTimestamp; + } + + /** + * Used for web ui to show when the HBCK checking report generated. + */ + public Instant getCheckingEndTimestamp() { + return checkingEndTimestamp; + } + + public void setCheckingEndTimestamp(Instant checkingEndTimestamp) { + this.checkingEndTimestamp = checkingEndTimestamp; + } + + /** + * This map contains the state of all hbck items. It maps from encoded region name to + * HbckRegionInfo structure. The information contained in HbckRegionInfo is used to detect and + * correct consistency (hdfs/meta/deployment) problems. + */ + public Map getRegionInfoMap() { + return regionInfoMap; + } + + public Set getDisabledTableRegions() { + return disabledTableRegions; + } + + public Set getSplitParentRegions() { + return splitParentRegions; + } + + /** + * The regions only opened on RegionServers, but no region info in meta. + */ + public Map getOrphanRegionsOnRS() { + return orphanRegionsOnRS; + } + + /** + * The regions have directory on FileSystem, but no region info in meta. + */ + public Map getOrphanRegionsOnFS() { + return orphanRegionsOnFS; + } + + /** + * The inconsistent regions. There are three case: case 1. Master thought this region opened, but + * no regionserver reported it. case 2. Master thought this region opened on Server1, but + * regionserver reported Server2 case 3. More than one regionservers reported opened this region + */ + public Map>> getInconsistentRegions() { + return inconsistentRegions; + } +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java index 064b2ef66425..d5d8f9e0a62b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java @@ -85,7 +85,7 @@ public class CatalogJanitor extends ScheduledChore { * Saved report from last hbase:meta scan to completion. May be stale if having trouble completing * scan. Check its date. */ - private volatile Report lastReport; + private volatile CatalogJanitorReport lastReport; public CatalogJanitor(final MasterServices services) { super("CatalogJanitor-" + services.getServerName().toShortString(), services, @@ -229,10 +229,10 @@ && cleanParent(e.getKey(), e.getValue()) /** * Scan hbase:meta. - * @return Return generated {@link Report} + * @return Return generated {@link CatalogJanitorReport} */ // will be override in tests. - protected Report scanForReport() throws IOException { + protected CatalogJanitorReport scanForReport() throws IOException { ReportMakingVisitor visitor = new ReportMakingVisitor(this.services); // Null tablename means scan all of meta. MetaTableAccessor.scanMetaForTableRegions(this.services.getConnection(), visitor, null); @@ -242,7 +242,7 @@ protected Report scanForReport() throws IOException { /** * @return Returns last published Report that comes of last successful scan of hbase:meta. */ - public Report getLastReport() { + public CatalogJanitorReport getLastReport() { return this.lastReport; } @@ -495,7 +495,7 @@ public static void main(String[] args) throws IOException { t.put(p); } MetaTableAccessor.scanMetaForTableRegions(connection, visitor, null); - Report report = visitor.getReport(); + CatalogJanitorReport report = visitor.getReport(); LOG.info(report != null ? report.toString() : "empty"); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/Report.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitorReport.java similarity index 99% rename from hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/Report.java rename to hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitorReport.java index b16429c229aa..6a9eb7e1b4ec 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/Report.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitorReport.java @@ -34,7 +34,7 @@ * Report made by ReportMakingVisitor */ @InterfaceAudience.Private -public class Report { +public class CatalogJanitorReport { private final long now = EnvironmentEdgeManager.currentTime(); // Keep Map of found split parents. These are candidates for cleanup. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java index 1e4ae5873adc..77410c3d91ce 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java @@ -76,7 +76,7 @@ public MetaFixer(MasterServices masterServices) { } public void fix() throws IOException { - Report report = this.masterServices.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = this.masterServices.getCatalogJanitor().getLastReport(); if (report == null) { LOG.info("CatalogJanitor has not generated a report yet; run 'catalogjanitor_run' in " + "shell or wait until CatalogJanitor chore runs."); @@ -93,7 +93,7 @@ public void fix() throws IOException { * If hole, it papers it over by adding a region in the filesystem and to hbase:meta. Does not * assign. */ - void fixHoles(Report report) { + void fixHoles(CatalogJanitorReport report) { final List> holes = report.getHoles(); if (holes.isEmpty()) { LOG.info("CatalogJanitor Report contains no holes to fix. Skipping."); @@ -229,7 +229,7 @@ private static List createMetaEntries(final MasterServices masterSer /** * Fix overlaps noted in CJ consistency report. */ - List fixOverlaps(Report report) throws IOException { + List fixOverlaps(CatalogJanitorReport report) throws IOException { List pidList = new ArrayList<>(); for (Set regions : calculateMerges(maxMergeCount, report.getOverlaps())) { RegionInfo[] regionsArray = regions.toArray(new RegionInfo[] {}); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java index 3fb45f99c9e8..a61c90106854 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java @@ -53,7 +53,7 @@ class ReportMakingVisitor implements MetaTableAccessor.CloseableVisitor { /** * Report is not done until after the close has been called. */ - private Report report = new Report(); + private CatalogJanitorReport report = new CatalogJanitorReport(); /** * RegionInfo from previous row. @@ -76,7 +76,7 @@ class ReportMakingVisitor implements MetaTableAccessor.CloseableVisitor { /** * Do not call until after {@link #close()}. Will throw a {@link RuntimeException} if you do. */ - Report getReport() { + CatalogJanitorReport getReport() { if (!this.closed) { throw new RuntimeException("Report not ready until after close()"); } diff --git a/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp b/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp index 69b95e1a118e..21128cea0471 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp @@ -20,7 +20,6 @@ <%@ page contentType="text/html;charset=UTF-8" import="java.time.Instant" import="java.time.ZoneId" - import="java.util.Date" import="java.util.List" import="java.util.Map" import="java.util.stream.Collectors" @@ -29,14 +28,17 @@ %> <%@ page import="org.apache.hadoop.fs.Path" %> <%@ page import="org.apache.hadoop.hbase.client.RegionInfo" %> -<%@ page import="org.apache.hadoop.hbase.master.HbckChore" %> +<%@ page import="org.apache.hadoop.hbase.master.hbck.HbckChore" %> +<%@ page import="org.apache.hadoop.hbase.master.hbck.HbckReport" %> <%@ page import="org.apache.hadoop.hbase.master.HMaster" %> <%@ page import="org.apache.hadoop.hbase.master.ServerManager" %> <%@ page import="org.apache.hadoop.hbase.ServerName" %> <%@ page import="org.apache.hadoop.hbase.util.Bytes" %> <%@ page import="org.apache.hadoop.hbase.util.Pair" %> <%@ page import="org.apache.hadoop.hbase.master.janitor.CatalogJanitor" %> -<%@ page import="org.apache.hadoop.hbase.master.janitor.Report" %> +<%@ page import="org.apache.hadoop.hbase.master.janitor.CatalogJanitorReport" %> +<%@ page import="java.util.Optional" %> +<%@ page import="org.apache.hadoop.hbase.util.EnvironmentEdgeManager" %> <% final String cacheParameterValue = request.getParameter("cache"); final HMaster master = (HMaster) getServletContext().getAttribute(HMaster.MASTER); @@ -55,26 +57,19 @@ } } HbckChore hbckChore = master.getHbckChore(); - Map>> inconsistentRegions = null; - Map orphanRegionsOnRS = null; - Map orphanRegionsOnFS = null; - long startTimestamp = 0; - long endTimestamp = 0; - if (hbckChore != null) { - inconsistentRegions = hbckChore.getInconsistentRegions(); - orphanRegionsOnRS = hbckChore.getOrphanRegionsOnRS(); - orphanRegionsOnFS = hbckChore.getOrphanRegionsOnFS(); - startTimestamp = hbckChore.getCheckingStartTimestamp(); - endTimestamp = hbckChore.getCheckingEndTimestamp(); - } - ZonedDateTime zdt = ZonedDateTime.ofInstant(Instant.ofEpochMilli(startTimestamp), - ZoneId.systemDefault()); - String iso8601start = startTimestamp == 0? "-1": zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME); - zdt = ZonedDateTime.ofInstant(Instant.ofEpochMilli(endTimestamp), - ZoneId.systemDefault()); - String iso8601end = startTimestamp == 0? "-1": zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME); + HbckReport hbckReport = hbckChore == null ? null : hbckChore.getLastReport(); + String hbckReportStartTime = Optional.ofNullable(hbckReport) + .map(HbckReport::getCheckingStartTimestamp) + .map(start -> ZonedDateTime.ofInstant(start, ZoneId.systemDefault())) + .map(zdt -> zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME)) + .orElse(null); + String hbckReportEndTime = Optional.ofNullable(hbckReport) + .map(HbckReport::getCheckingEndTimestamp) + .map(start -> ZonedDateTime.ofInstant(start, ZoneId.systemDefault())) + .map(zdt -> zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME)) + .orElse(null); CatalogJanitor cj = master.getCatalogJanitor(); - Report report = cj == null? null: cj.getLastReport(); + CatalogJanitorReport cjReport = cj == null? null: cj.getLastReport(); final ServerManager serverManager = master.getServerManager(); %> @@ -109,20 +104,22 @@

HBCK Chore Report

- <% if (hbckChore.isDisabled()) { %> + <% if (hbckChore == null) { %> + HBCK chore has not yet initialized. Try again later. + <% } else if (hbckChore.isDisabled()) { %> HBCK chore is currently disabled. Set hbase.master.hbck.chore.interval > 0 in the config & do a rolling-restart to enable it. - <% } else if (startTimestamp == 0 && endTimestamp == 0){ %> - No report created. - <% } else if (startTimestamp > 0 && endTimestamp == 0){ %> - Checking started at <%= iso8601start %>. Please wait for checking to generate a new sub-report. + <% } else if (hbckReport == null) { %> + No Report created. + <% } else if (hbckReportStartTime != null && hbckReportEndTime == null) { %> + Checking started at <%= hbckReportStartTime %>. Please wait for checking to generate a new sub-report. <% } else { %> - Checking started at <%= iso8601start %> and generated report at <%= iso8601end %>. + Checking started at <%= hbckReportStartTime %> and generated catalogJanitorReport at <%= hbckReportEndTime %>. <% } %>

- <% if (inconsistentRegions != null && inconsistentRegions.size() > 0) { %> + <% if (hbckReport != null && hbckReport.getInconsistentRegions().size() > 0) { %>

Inconsistent Regions

@@ -145,7 +142,7 @@ Location in META Reported Online RegionServers - <% for (Map.Entry>> entry : inconsistentRegions.entrySet()) {%> + <% for (Map.Entry>> entry : hbckReport.getInconsistentRegions().entrySet()) { %> <%= entry.getKey() %> <%= formatServerName(master, serverManager, entry.getValue().getFirst()) %> @@ -153,11 +150,11 @@ collect(Collectors.joining(", ")) %> <% } %> -

<%= inconsistentRegions.size() %> region(s) in set.

+

<%= hbckReport.getInconsistentRegions().size() %> region(s) in set.

<% } %> - <% if (orphanRegionsOnRS != null && orphanRegionsOnRS.size() > 0) { %> + <% if (hbckReport != null && hbckReport.getOrphanRegionsOnRS().size() > 0) { %>

Orphan Regions on RegionServer

@@ -169,17 +166,17 @@ Region Name Reported Online RegionServer - <% for (Map.Entry entry : orphanRegionsOnRS.entrySet()) { %> + <% for (Map.Entry entry : hbckReport.getOrphanRegionsOnRS().entrySet()) { %> <%= entry.getKey() %> <%= formatServerName(master, serverManager, entry.getValue()) %> <% } %> -

<%= orphanRegionsOnRS.size() %> region(s) in set.

+

<%= hbckReport.getOrphanRegionsOnRS().size() %> region(s) in set.

<% } %> - <% if (orphanRegionsOnFS != null && orphanRegionsOnFS.size() > 0) { %> + <% if (hbckReport != null && hbckReport.getOrphanRegionsOnFS().size() > 0) { %>

Orphan Regions on FileSystem

@@ -200,41 +197,41 @@ Region Encoded Name FileSystem Path - <% for (Map.Entry entry : orphanRegionsOnFS.entrySet()) { %> + <% for (Map.Entry entry : hbckReport.getOrphanRegionsOnFS().entrySet()) { %> <%= entry.getKey() %> <%= entry.getValue() %> <% } %> -

<%= orphanRegionsOnFS.size() %> region(s) in set.

+

<%= hbckReport.getOrphanRegionsOnFS().size() %> region(s) in set.

<% } %> <% - zdt = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), - ZoneId.systemDefault()); - String iso8601Now = zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME); - String iso8601reportTime = "-1"; - if (report != null) { - zdt = ZonedDateTime.ofInstant(Instant.ofEpochMilli(report.getCreateTime()), - ZoneId.systemDefault()); - iso8601reportTime = zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME); - } + Instant nowInstant = Instant.ofEpochMilli(EnvironmentEdgeManager.currentTime()); + ZonedDateTime nowZdt = ZonedDateTime.ofInstant(nowInstant, ZoneId.systemDefault()); + String iso8601Now = nowZdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME); + String cjReportTime = Optional.ofNullable(cjReport) + .map(CatalogJanitorReport::getCreateTime) + .map(Instant::ofEpochMilli) + .map(start -> ZonedDateTime.ofInstant(start, ZoneId.systemDefault())) + .map(zdt -> zdt.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME)) + .orElse(null); %>

CatalogJanitor hbase:meta Consistency Issues

- <% if (report != null) { %> - Report created: <%= iso8601reportTime %> (now=<%= iso8601Now %>).

+ <% if (cjReport != null) { %> + Report created: <%= cjReportTime %> (now=<%= iso8601Now %>).

<% } else { %> - No report created. + No catalogJanitorReport created. <% } %>
- <% if (report != null && !report.isEmpty()) { %> - <% if (!report.getHoles().isEmpty()) { %> + <% if (cjReport != null && !cjReport.isEmpty()) { %> + <% if (!cjReport.getHoles().isEmpty()) { %>

Holes

@@ -245,17 +242,17 @@ RegionInfo RegionInfo - <% for (Pair p : report.getHoles()) { %> + <% for (Pair p : cjReport.getHoles()) { %> <%= p.getFirst().getRegionNameAsString() %> <%= p.getSecond().getRegionNameAsString() %> <% } %> -

<%= report.getHoles().size() %> hole(s).

+

<%= cjReport.getHoles().size() %> hole(s).

<% } %> - <% if (!report.getOverlaps().isEmpty()) { %> + <% if (!cjReport.getOverlaps().isEmpty()) { %>

Overlaps

@@ -272,14 +269,14 @@ RegionInfo Other RegionInfo - <% for (Pair p : report.getOverlaps()) { %> + <% for (Pair p : cjReport.getOverlaps()) { %> - <% if (report.getMergedRegions().containsKey(p.getFirst())) { %> + <% if (cjReport.getMergedRegions().containsKey(p.getFirst())) { %> <%= p.getFirst().getRegionNameAsString() %> <% } else { %> <%= p.getFirst().getRegionNameAsString() %> <% } %> - <% if (report.getMergedRegions().containsKey(p.getSecond())) { %> + <% if (cjReport.getMergedRegions().containsKey(p.getSecond())) { %> <%= p.getSecond().getRegionNameAsString() %> <% } else { %> <%= p.getSecond().getRegionNameAsString() %> @@ -287,10 +284,10 @@ <% } %> -

<%= report.getOverlaps().size() %> overlap(s).

+

<%= cjReport.getOverlaps().size() %> overlap(s).

<% } %> - <% if (!report.getUnknownServers().isEmpty()) { %> + <% if (!cjReport.getUnknownServers().isEmpty()) { %>

Unknown Servers

@@ -316,17 +313,17 @@ RegionInfo ServerName - <% for (Pair p: report.getUnknownServers()) { %> + <% for (Pair p: cjReport.getUnknownServers()) { %> <%= p.getFirst().getRegionNameAsString() %> <%= p.getSecond() %> <% } %> -

<%= report.getUnknownServers().size() %> unknown servers(s).

+

<%= cjReport.getUnknownServers().size() %> unknown servers(s).

<% } %> - <% if (!report.getEmptyRegionInfo().isEmpty()) { %> + <% if (!cjReport.getEmptyRegionInfo().isEmpty()) { %>

Empty info:regioninfo

@@ -336,13 +333,13 @@ Row - <% for (byte [] row: report.getEmptyRegionInfo()) { %> + <% for (byte [] row: cjReport.getEmptyRegionInfo()) { %> <%= Bytes.toStringBinary(row) %> <% } %> -

<%= report.getEmptyRegionInfo().size() %> emptyRegionInfo(s).

+

<%= cjReport.getEmptyRegionInfo().size() %> emptyRegionInfo(s).

<% } %> <% } %> diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java index 858716a009db..7656ab225012 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHbck.java @@ -19,15 +19,18 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.time.Instant; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.CoprocessorEnvironment; @@ -42,6 +45,8 @@ import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; +import org.apache.hadoop.hbase.master.hbck.HbckChore; +import org.apache.hadoop.hbase.master.hbck.HbckReport; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.procedure.TableProcedureInterface; import org.apache.hadoop.hbase.procedure2.Procedure; @@ -302,15 +307,14 @@ public void testScheduleSCP() throws Exception { @Test public void testRunHbckChore() throws Exception { HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster(); - long endTimestamp = master.getHbckChore().getCheckingEndTimestamp(); + HbckChore hbckChore = master.getHbckChore(); + Instant endTimestamp = Optional.ofNullable(hbckChore.getLastReport()) + .map(HbckReport::getCheckingEndTimestamp).orElse(Instant.EPOCH); Hbck hbck = getHbck(); - boolean ran = false; - while (!ran) { - ran = hbck.runHbckChore(); - if (ran) { - assertTrue(master.getHbckChore().getCheckingEndTimestamp() > endTimestamp); - } - } + TEST_UTIL.waitFor(TimeUnit.MINUTES.toMillis(5), hbck::runHbckChore); + HbckReport report = hbckChore.getLastReport(); + assertNotNull(report); + assertTrue(report.getCheckingEndTimestamp().isAfter(endTimestamp)); } public static class FailingSplitAfterMetaUpdatedMasterObserver diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterChoreScheduled.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterChoreScheduled.java index 9b84bb729aec..0d7525a33a59 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterChoreScheduled.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterChoreScheduled.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; import org.apache.hadoop.hbase.master.cleaner.LogCleaner; import org.apache.hadoop.hbase.master.cleaner.ReplicationBarrierCleaner; +import org.apache.hadoop.hbase.master.hbck.HbckChore; import org.apache.hadoop.hbase.master.janitor.CatalogJanitor; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestHbckChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestHbckChore.java index 908101e07e51..1939544ce706 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestHbckChore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestHbckChore.java @@ -19,11 +19,16 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.Future; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -33,8 +38,9 @@ import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.TableState; -import org.apache.hadoop.hbase.master.HbckChore; import org.apache.hadoop.hbase.master.TableStateManager; +import org.apache.hadoop.hbase.master.hbck.HbckChore; +import org.apache.hadoop.hbase.master.hbck.HbckReport; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -75,7 +81,7 @@ public void testForMeta() { hbckChore.choreForTesting(); Map>> inconsistentRegions = - hbckChore.getInconsistentRegions(); + hbckChore.getLastReport().getInconsistentRegions(); // Test for case1: Master thought this region opened, but no regionserver reported it. assertTrue(inconsistentRegions.containsKey(metaRegionName)); @@ -88,7 +94,7 @@ public void testForMeta() { // Reported right region location. Then not in problematic regions. am.reportOnlineRegions(locationInMeta, Collections.singleton(metaRegionNameAsBytes)); hbckChore.choreForTesting(); - inconsistentRegions = hbckChore.getInconsistentRegions(); + inconsistentRegions = hbckChore.getLastReport().getInconsistentRegions(); assertFalse(inconsistentRegions.containsKey(metaRegionName)); } @@ -107,7 +113,7 @@ public void testForUserTable() throws Exception { // Test for case1: Master thought this region opened, but no regionserver reported it. hbckChore.choreForTesting(); Map>> inconsistentRegions = - hbckChore.getInconsistentRegions(); + hbckChore.getLastReport().getInconsistentRegions(); assertTrue(inconsistentRegions.containsKey(regionName)); Pair> pair = inconsistentRegions.get(regionName); ServerName locationInMeta = pair.getFirst(); @@ -122,7 +128,7 @@ public void testForUserTable() throws Exception { serverNames.stream().filter(s -> !s.equals(tempLocationInMeta)).findFirst().get(); am.reportOnlineRegions(anotherServer, Collections.singleton(hri.getRegionName())); hbckChore.choreForTesting(); - inconsistentRegions = hbckChore.getInconsistentRegions(); + inconsistentRegions = hbckChore.getLastReport().getInconsistentRegions(); assertTrue(inconsistentRegions.containsKey(regionName)); pair = inconsistentRegions.get(regionName); locationInMeta = pair.getFirst(); @@ -134,7 +140,7 @@ public void testForUserTable() throws Exception { // Test for case3: More than one regionservers reported opened this region. am.reportOnlineRegions(locationInMeta, Collections.singleton(hri.getRegionName())); hbckChore.choreForTesting(); - inconsistentRegions = hbckChore.getInconsistentRegions(); + inconsistentRegions = hbckChore.getLastReport().getInconsistentRegions(); assertTrue(inconsistentRegions.containsKey(regionName)); pair = inconsistentRegions.get(regionName); locationInMeta = pair.getFirst(); @@ -146,7 +152,7 @@ public void testForUserTable() throws Exception { // Reported right region location, then not in inconsistent regions. am.reportOnlineRegions(anotherServer, Collections.EMPTY_SET); hbckChore.choreForTesting(); - inconsistentRegions = hbckChore.getInconsistentRegions(); + inconsistentRegions = hbckChore.getLastReport().getInconsistentRegions(); assertFalse(inconsistentRegions.containsKey(regionName)); } @@ -164,7 +170,7 @@ public void testForDisabledTable() throws Exception { hbckChore.choreForTesting(); Map>> inconsistentRegions = - hbckChore.getInconsistentRegions(); + hbckChore.getLastReport().getInconsistentRegions(); assertTrue(inconsistentRegions.containsKey(regionName)); Pair> pair = inconsistentRegions.get(regionName); ServerName locationInMeta = pair.getFirst(); @@ -177,7 +183,7 @@ public void testForDisabledTable() throws Exception { Mockito.when(tableStateManager.isTableState(tableName, TableState.State.DISABLED)) .thenReturn(true); hbckChore.choreForTesting(); - inconsistentRegions = hbckChore.getInconsistentRegions(); + inconsistentRegions = hbckChore.getLastReport().getInconsistentRegions(); assertFalse(inconsistentRegions.containsKey(regionName)); } @@ -196,7 +202,7 @@ public void testForSplitParent() throws Exception { hbckChore.choreForTesting(); Map>> inconsistentRegions = - hbckChore.getInconsistentRegions(); + hbckChore.getLastReport().getInconsistentRegions(); assertFalse(inconsistentRegions.containsKey(regionName)); } @@ -207,33 +213,35 @@ public void testOrphanRegionsOnFS() throws Exception { Configuration conf = util.getConfiguration(); hbckChore.choreForTesting(); - assertEquals(0, hbckChore.getOrphanRegionsOnFS().size()); + assertEquals(0, hbckChore.getLastReport().getOrphanRegionsOnFS().size()); HRegion.createRegionDir(conf, regionInfo, CommonFSUtils.getRootDir(conf)); hbckChore.choreForTesting(); - assertEquals(1, hbckChore.getOrphanRegionsOnFS().size()); - assertTrue(hbckChore.getOrphanRegionsOnFS().containsKey(regionInfo.getEncodedName())); + assertEquals(1, hbckChore.getLastReport().getOrphanRegionsOnFS().size()); + assertTrue( + hbckChore.getLastReport().getOrphanRegionsOnFS().containsKey(regionInfo.getEncodedName())); FSUtils.deleteRegionDir(conf, new HRegionInfo(regionInfo)); hbckChore.choreForTesting(); - assertEquals(0, hbckChore.getOrphanRegionsOnFS().size()); + assertEquals(0, hbckChore.getLastReport().getOrphanRegionsOnFS().size()); } @Test public void testChoreDisable() { // The way to disable to chore is to set hbase.master.hbck.chore.interval <= 0 // When the interval is > 0, the chore should run. - long lastRunTime = hbckChore.getCheckingEndTimestamp(); + Instant lastRunTime = Optional.ofNullable(hbckChore.getLastReport()) + .map(HbckReport::getCheckingEndTimestamp).orElse(null); hbckChore.choreForTesting(); - boolean ran = lastRunTime != hbckChore.getCheckingEndTimestamp(); - assertTrue(ran); + Instant thisRunTime = Optional.ofNullable(hbckChore.getLastReport()) + .map(HbckReport::getCheckingEndTimestamp).orElse(null); + assertNotNull(thisRunTime); + assertNotEquals(lastRunTime, thisRunTime); // When the interval <= 0, the chore shouldn't run master.getConfiguration().setInt("hbase.master.hbck.chore.interval", 0); HbckChore hbckChoreWithChangedConf = new HbckChore(master); - lastRunTime = hbckChoreWithChangedConf.getCheckingEndTimestamp(); hbckChoreWithChangedConf.choreForTesting(); - ran = lastRunTime != hbckChoreWithChangedConf.getCheckingEndTimestamp(); - assertFalse(ran); + assertNull(hbckChoreWithChangedConf.getLastReport()); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java index 759bd2d7bb07..09713e5bde23 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java @@ -312,7 +312,7 @@ public void testScanDoesNotCleanRegionsWithExistingParents() throws Exception { final Map mergedRegions = new TreeMap<>(); CatalogJanitor spy = spy(this.janitor); - Report report = new Report(); + CatalogJanitorReport report = new CatalogJanitorReport(); report.count = 10; report.mergedRegions.putAll(mergedRegions); report.splitParents.putAll(splitParents); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorCluster.java index 885a0e6ce5d2..a0af5cdaeb9b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorCluster.java @@ -102,7 +102,7 @@ public void testConsistency() throws IOException { RegionStateStore regionStateStore = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionStateStore(); janitor.scan(); - Report report = janitor.getLastReport(); + CatalogJanitorReport report = janitor.getLastReport(); // Assert no problems. assertTrue(report.isEmpty()); // Now remove first region in table t2 to see if catalogjanitor scan notices. @@ -230,7 +230,7 @@ private static byte[] incrementRow(byte[] row) { public void testHoles() throws IOException { CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().getCatalogJanitor(); - Report report = janitor.getLastReport(); + CatalogJanitorReport report = janitor.getLastReport(); // Assert no problems. assertTrue(report.isEmpty()); // Verify start and end region holes @@ -245,7 +245,7 @@ public void testHoles() throws IOException { private void fixHoles(CatalogJanitor janitor) throws IOException { MetaFixer metaFixer = new MetaFixer(TEST_UTIL.getHBaseCluster().getMaster()); janitor.scan(); - Report report = janitor.getLastReport(); + CatalogJanitorReport report = janitor.getLastReport(); // Verify total number of holes, 2 in t1 and t2 each and one in t3 assertEquals("Number of holes are not matching", 5, report.getHoles().size()); metaFixer.fix(); @@ -300,7 +300,7 @@ private void verifyCornerHoles(CatalogJanitor janitor, TableName tableName) thro private LinkedList> getHoles(CatalogJanitor janitor, TableName tableName) throws IOException { janitor.scan(); - Report lastReport = janitor.getLastReport(); + CatalogJanitorReport lastReport = janitor.getLastReport(); assertFalse(lastReport.isEmpty()); LinkedList> holes = new LinkedList<>(); for (Pair hole : lastReport.getHoles()) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestMetaFixer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestMetaFixer.java index a79513051d15..b2533b99e4cf 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestMetaFixer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestMetaFixer.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.master.janitor; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -39,12 +40,13 @@ import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.master.HMaster; -import org.apache.hadoop.hbase.master.HbckChore; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; import org.apache.hadoop.hbase.master.assignment.GCMultipleMergedRegionsProcedure; import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure; import org.apache.hadoop.hbase.master.assignment.RegionStates; +import org.apache.hadoop.hbase.master.hbck.HbckChore; +import org.apache.hadoop.hbase.master.hbck.HbckReport; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; @@ -94,7 +96,7 @@ private void testPlugsHolesWithReadReplicaInternal(final TableName tn, final int MasterServices services = TEST_UTIL.getHBaseCluster().getMaster(); int initialSize = services.getAssignmentManager().getRegionStates().getRegionStates().size(); services.getCatalogJanitor().scan(); - Report report = services.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = services.getCatalogJanitor().getLastReport(); assertTrue(report.isEmpty()); int originalCount = ris.size(); // Remove first, last and middle region. See if hole gets plugged. Table has 26 * replicaCount @@ -152,7 +154,7 @@ public void testOneRegionTable() throws IOException { services.getCatalogJanitor().scan(); deleteRegion(services, ris.get(0)); services.getCatalogJanitor().scan(); - Report report = services.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = services.getCatalogJanitor().getLastReport(); ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn); assertTrue(ris.isEmpty()); MetaFixer fixer = new MetaFixer(services); @@ -182,7 +184,7 @@ private void testOverlapCommon(final TableName tn) throws Exception { assertTrue(ris.size() > 5); HMaster services = TEST_UTIL.getHBaseCluster().getMaster(); services.getCatalogJanitor().scan(); - Report report = services.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = services.getCatalogJanitor().getLastReport(); assertTrue(report.isEmpty()); // Make a simple overlap spanning second and third region. makeOverlap(services, ris.get(1), ris.get(3)); @@ -199,7 +201,7 @@ public void testOverlap() throws Exception { CatalogJanitor cj = services.getCatalogJanitor(); cj.scan(); - Report report = cj.getLastReport(); + CatalogJanitorReport report = cj.getLastReport(); assertEquals(6, report.getOverlaps().size()); assertEquals(1, MetaFixer.calculateMerges(10, report.getOverlaps()).size()); MetaFixer fixer = new MetaFixer(services); @@ -235,11 +237,13 @@ public void testOverlap() throws Exception { // No orphan regions on FS hbckChore.choreForTesting(); - assertEquals(0, hbckChore.getOrphanRegionsOnFS().size()); + HbckReport hbckReport = hbckChore.getLastReport(); + assertNotNull(hbckReport); + assertEquals(0, hbckReport.getOrphanRegionsOnFS().size()); // No holes reported. cj.scan(); - final Report postReport = cj.getLastReport(); + final CatalogJanitorReport postReport = cj.getLastReport(); assertTrue(postReport.isEmpty()); } @@ -253,7 +257,7 @@ public void testMultipleTableOverlaps() throws Exception { HMaster services = TEST_UTIL.getHBaseCluster().getMaster(); services.getCatalogJanitor().scan(); - Report report = services.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = services.getCatalogJanitor().getLastReport(); assertTrue(report.isEmpty()); // Make a simple overlap for t1 @@ -286,7 +290,7 @@ public void testOverlapWithSmallMergeCount() throws Exception { HMaster services = TEST_UTIL.getHBaseCluster().getMaster(); CatalogJanitor cj = services.getCatalogJanitor(); cj.scan(); - Report report = cj.getLastReport(); + CatalogJanitorReport report = cj.getLastReport(); assertEquals(6, report.getOverlaps().size()); assertEquals(2, MetaFixer.calculateMerges(5, report.getOverlaps()).size()); @@ -309,7 +313,7 @@ public void testOverlapWithSmallMergeCount() throws Exception { HBaseTestingUtility.await(200, () -> { try { cj.scan(); - final Report postReport = cj.getLastReport(); + final CatalogJanitorReport postReport = cj.getLastReport(); RegionStates regionStates = am.getRegionStates(); // Make sure that two merged regions are opened and GCs are done. @@ -352,7 +356,7 @@ public void testOverlapWithSmallMergeCount() throws Exception { // No holes reported. cj.scan(); - final Report postReport = cj.getLastReport(); + final CatalogJanitorReport postReport = cj.getLastReport(); assertTrue(postReport.isEmpty()); } finally { @@ -377,7 +381,7 @@ public void testMergeWithMergedChildRegion() throws Exception { HMaster services = TEST_UTIL.getHBaseCluster().getMaster(); CatalogJanitor cj = services.getCatalogJanitor(); cj.scan(); - Report report = cj.getLastReport(); + CatalogJanitorReport report = cj.getLastReport(); assertTrue(report.isEmpty()); RegionInfo overlapRegion = makeOverlap(services, ris.get(1), ris.get(2)); @@ -438,7 +442,7 @@ public void testOverlapWithMergeOfNonContiguous() throws Exception { assertTrue(ris.size() > 5); MasterServices services = TEST_UTIL.getHBaseCluster().getMaster(); services.getCatalogJanitor().scan(); - Report report = services.getCatalogJanitor().getLastReport(); + CatalogJanitorReport report = services.getCatalogJanitor().getLastReport(); assertTrue(report.isEmpty()); // Make a simple overlap spanning second and third region. makeOverlap(services, ris.get(1), ris.get(5)); @@ -462,7 +466,7 @@ public void testOverlapWithMergeOfNonContiguous() throws Exception { HBaseTestingUtility.await(10, () -> { try { services.getCatalogJanitor().scan(); - final Report postReport = services.getCatalogJanitor().getLastReport(); + final CatalogJanitorReport postReport = services.getCatalogJanitor().getLastReport(); return postReport.isEmpty(); } catch (Exception e) { throw new RuntimeException(e);