Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HBASE-24273 HBCK's "Orphan Regions on FileSystem" reports regions wit… #1613

Merged
merged 1 commit into from
May 5, 2020
Merged

HBASE-24273 HBCK's "Orphan Regions on FileSystem" reports regions wit… #1613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.ScheduledChore;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.client.RegionInfo;
Expand Down Expand Up @@ -134,7 +135,7 @@ protected synchronized void chore() {
loadRegionsFromInMemoryState();
loadRegionsFromRSReport();
try {
loadRegionsFromFS();
loadRegionsFromFS(scanForMergedParentRegions());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to make it a bit more generic and have "loadRegionsFromMeta()" function (similar to loadRegionsFromInMemoryState/loadRegionsFromRSReport) as then you'd have another source to compare against - hbase:meta. Then loadRegionsFromFS() would check against that state to see if a region is merged or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is the idea. scanForMergedParentRegions() loads regions from meta. For merge, it is a bit special, the parent regions are deleted from meta already, the only bit left is the merge qualifers in the child region.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does InMemoryState not have the needed merge info in it? If not, maybe it should.

The CatalogJanitor is what manages when merge references are let go so this scan of meta is probably necessary.

To the @timoha point, are there other places in hbckchore where we need currrent picture of hbase:meta?

Copy link
Contributor Author

@huaxiangsun huaxiangsun May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saintstack Yeah the in-memory state/meta row for merged parents are let go at early state of MergeRegionsProcedures.

@timoha Per Stack's comments about the source of truth is the in-memory database (meta/procedure store are ways to recover in-memory databse since they are persistent).

At this moment, there is no other usage of regions from meta in hbck chore, the merged parents info is a special case, they are columns from the child region, the only source of truth for merged parents. We can maintain the in-memory hashset for merged parent if meta scan is too costly, which can be addressed later. In case of the future requirements, scanForMergedParentRegions() can be modified to get more info from meta.

} catch (IOException e) {
LOG.warn("Failed to load the regions from filesystem", e);
}
Expand Down Expand Up @@ -187,6 +188,31 @@ private void saveCheckResultToSnapshot() {
}
}

/**
* Scan hbase:meta to get set of merged parent regions, this is a very heavy scan.
*
* @return Return generated {@link HashSet}
*/
private HashSet<String> scanForMergedParentRegions() throws IOException {
HashSet<String> mergedParentRegions = new HashSet<>();
// Null tablename means scan all of meta.
MetaTableAccessor.scanMetaForTableRegions(this.master.getConnection(),
r -> {
List<RegionInfo> mergeParents = MetaTableAccessor.getMergeRegions(r.rawCells());
if (mergeParents != null) {
for (RegionInfo mergeRegion : mergeParents) {
if (mergeRegion != null) {
// This region is already being merged
mergedParentRegions.add(mergeRegion.getEncodedName());
}
}
}
return true;
},
null);
return mergedParentRegions;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, may be no way around it given the scan for merged parents is so specialized.

This looks good.

private void loadRegionsFromInMemoryState() {
List<RegionState> regionStates =
master.getAssignmentManager().getRegionStates().getRegionStates();
Expand Down Expand Up @@ -256,7 +282,7 @@ private void loadRegionsFromRSReport() {
}
}

private void loadRegionsFromFS() throws IOException {
private void loadRegionsFromFS(final HashSet<String> mergedParentRegions) throws IOException {
Path rootDir = master.getMasterFileSystem().getRootDir();
FileSystem fs = master.getMasterFileSystem().getFileSystem();

Expand All @@ -271,12 +297,12 @@ private void loadRegionsFromFS() throws IOException {
continue;
}
HbckRegionInfo hri = regionInfoMap.get(encodedRegionName);
if (hri == null) {
// 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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's not in in-memory database but it is in merged regions, that seems like a problem as well and should be reported?

Copy link
Contributor Author

@huaxiangsun huaxiangsun Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to the following case?

if (hri == null && mergedParentRegions.contains(encodedRegionName)) {}
This is valid for merged parent regions as of today.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, merge parents are NOT in in-memory state because they are not active. We just have this background cleaner task that is doing janitorial work on hbase:meta cleaning up meta and filesystem....

orphanRegionsOnFS.put(encodedRegionName, regionDir);
continue;
}
HbckRegionInfo.HdfsEntry hdfsEntry = new HbckRegionInfo.HdfsEntry(regionDir);
hri.setHdfsEntry(hdfsEntry);
}
numRegions += regionDirs.size();
}
Expand Down
12 changes: 9 additions & 3 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
*/
package org.apache.hadoop.hbase.client;

import java.io.IOException;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ZooKeeperConnectionException;
import org.mockito.Mockito;

/**
Expand All @@ -39,12 +39,18 @@ public class HConnectionTestingUtility {
* probably not what you want.
* @param conf configuration
* @return ConnectionImplementation object for <code>conf</code>
* @throws ZooKeeperConnectionException
*/
public static Connection getMockedConnection(final Configuration conf)
throws ZooKeeperConnectionException {
throws IOException {
Connection connection = Mockito.mock(Connection.class);
Mockito.when(connection.getConfiguration()).thenReturn(conf);

// Some test cases need Mock of getTable and getScanner
Table t = Mockito.mock(Table.class);
Mockito.when(connection.getTable(Mockito.any())).thenReturn(t);
ResultScanner rs = Mockito.mock(ResultScanner.class);
Mockito.when(t.getScanner((Scan)Mockito.any())).thenReturn(rs);

return connection;
}
}
10 changes: 8 additions & 2 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.testclassification.MasterTests;
Expand Down Expand Up @@ -147,10 +148,12 @@ private static void makeOverlap(MasterServices services, RegionInfo a, RegionInf
@Test
public void testOverlap() throws Exception {
TableName tn = TableName.valueOf(this.name.getMethodName());
TEST_UTIL.createMultiRegionTable(tn, HConstants.CATALOG_FAMILY);
Table t = TEST_UTIL.createMultiRegionTable(tn, HConstants.CATALOG_FAMILY);
TEST_UTIL.loadTable(t, HConstants.CATALOG_FAMILY);
List<RegionInfo> ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn);
assertTrue(ris.size() > 5);
MasterServices services = TEST_UTIL.getHBaseCluster().getMaster();
HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
HbckChore hbckChore = services.getHbckChore();
services.getCatalogJanitor().scan();
CatalogJanitor.Report report = services.getCatalogJanitor().getLastReport();
assertTrue(report.isEmpty());
Expand All @@ -174,6 +177,9 @@ public void testOverlap() throws Exception {
throw new RuntimeException(e);
}
});

hbckChore.chore();
assertEquals(0, hbckChore.getOrphanRegionsOnFS().size());
}

/**
Expand Down