You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/03/01 09:49:26 UTC
[GitHub] [hbase] karthikhw opened a new pull request #1230: Backport
HBASE-23553 to branch-2.1
karthikhw opened a new pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230
HBASE-23553 Snapshot referenced data files are deleted in some case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] busbey commented on issue #1230: Backport HBASE-23553 to
branch-2.1
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593963017
If there's test coverage then how do I run a unit test to get a failure prior to the fix?
On the original backport I applied just the test changes and then ran those tests. They passed. What am I missing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] busbey closed pull request #1230: Backport HBASE-23553 to
branch-2.1
Posted by GitBox <gi...@apache.org>.
busbey closed pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386496544
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -303,4 +312,131 @@ private static void verifyRow(Result result) throws IOException {
}
}
+ @Test
+ public void testMergeRegion() throws Exception {
+ setupCluster();
+ TableName tableName = TableName.valueOf("testMergeRegion");
+ String snapshotName = tableName.getNameAsString() + "_snapshot";
+ Configuration conf = UTIL.getConfiguration();
+ Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir();
+ long timeout = 20000; // 20s
+ try (Admin admin = UTIL.getAdmin()) {
+ List<String> serverList = admin.getRegionServers().stream().map(sn -> sn.getServerName())
Review comment:
Unused
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386502643
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -303,4 +312,131 @@ private static void verifyRow(Result result) throws IOException {
}
}
+ @Test
+ public void testMergeRegion() throws Exception {
+ setupCluster();
+ TableName tableName = TableName.valueOf("testMergeRegion");
+ String snapshotName = tableName.getNameAsString() + "_snapshot";
+ Configuration conf = UTIL.getConfiguration();
+ Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir();
+ long timeout = 20000; // 20s
+ try (Admin admin = UTIL.getAdmin()) {
+ List<String> serverList = admin.getRegionServers().stream().map(sn -> sn.getServerName())
+ .collect(Collectors.toList());
+ // create table with 3 regions
+ Table table = UTIL.createTable(tableName, FAMILIES, 1, bbb, yyy, 3);
+ List<RegionInfo> regions = admin.getRegions(tableName);
+ Assert.assertEquals(3, regions.size());
+ RegionInfo region0 = regions.get(0);
+ RegionInfo region1 = regions.get(1);
+ RegionInfo region2 = regions.get(2);
+ // put some data in the table
+ UTIL.loadTable(table, FAMILIES);
+ admin.flush(tableName);
+ // wait flush is finished
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (RegionInfo region : regions) {
+ Path regionDir = new Path(tableDir, region.getEncodedName());
+ for (Path familyDir : FSUtils.getFamilyDirs(fs, regionDir)) {
+ if (fs.listStatus(familyDir).length != 1) {
+ return false;
+ }
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Failed check if flush is finished", e);
+ return false;
+ }
+ });
+ // merge 2 regions
+ admin.mergeRegionsAsync(region0.getEncodedNameAsBytes(), region1.getEncodedNameAsBytes(),
+ true);
+ UTIL.waitFor(timeout, () -> admin.getRegions(tableName).size() == 2);
+ List<RegionInfo> mergedRegions = admin.getRegions(tableName);
+ RegionInfo mergedRegion =
+ mergedRegions.get(0).getEncodedName().equals(region2.getEncodedName())
+ ? mergedRegions.get(1)
+ : mergedRegions.get(0);
+ // snapshot
+ admin.snapshot(snapshotName, tableName);
+ Assert.assertEquals(1, admin.listSnapshots().size());
+ // major compact
+ admin.majorCompactRegion(mergedRegion.getRegionName());
+ // wait until merged region has no reference
+ UTIL.waitFor(timeout, () -> {
+ try {
+ for (RegionServerThread regionServerThread : UTIL.getMiniHBaseCluster()
+ .getRegionServerThreads()) {
+ HRegionServer regionServer = regionServerThread.getRegionServer();
+ for (HRegion subRegion : regionServer.getRegions(tableName)) {
+ if (subRegion.getRegionInfo().getEncodedName()
+ .equals(mergedRegion.getEncodedName())) {
+ regionServer.getCompactedHFilesDischarger().chore();
+ }
+ }
+ }
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ HRegionFileSystem regionFs = HRegionFileSystem
+ .openRegionFromFileSystem(UTIL.getConfiguration(), fs, tableDir, mergedRegion, true);
+ return !regionFs.hasReferences(admin.getDescriptor(tableName));
+ } catch (IOException e) {
+ LOG.warn("Failed check merged region has no reference", e);
+ return false;
+ }
+ });
+ // run catalog janitor to clean and wait for parent regions are archived
Review comment:
Nit: IIRC, we call the input to a merge the "daughters" of a merge which come together to make one "parent". It's like split (e.g one parent, two daughters), but in reverse.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386504683
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -303,4 +312,131 @@ private static void verifyRow(Result result) throws IOException {
}
}
+ @Test
+ public void testMergeRegion() throws Exception {
+ setupCluster();
+ TableName tableName = TableName.valueOf("testMergeRegion");
+ String snapshotName = tableName.getNameAsString() + "_snapshot";
+ Configuration conf = UTIL.getConfiguration();
+ Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir();
+ long timeout = 20000; // 20s
+ try (Admin admin = UTIL.getAdmin()) {
+ List<String> serverList = admin.getRegionServers().stream().map(sn -> sn.getServerName())
+ .collect(Collectors.toList());
+ // create table with 3 regions
+ Table table = UTIL.createTable(tableName, FAMILIES, 1, bbb, yyy, 3);
+ List<RegionInfo> regions = admin.getRegions(tableName);
+ Assert.assertEquals(3, regions.size());
+ RegionInfo region0 = regions.get(0);
+ RegionInfo region1 = regions.get(1);
+ RegionInfo region2 = regions.get(2);
+ // put some data in the table
+ UTIL.loadTable(table, FAMILIES);
+ admin.flush(tableName);
+ // wait flush is finished
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (RegionInfo region : regions) {
+ Path regionDir = new Path(tableDir, region.getEncodedName());
+ for (Path familyDir : FSUtils.getFamilyDirs(fs, regionDir)) {
+ if (fs.listStatus(familyDir).length != 1) {
+ return false;
+ }
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Failed check if flush is finished", e);
+ return false;
+ }
+ });
+ // merge 2 regions
+ admin.mergeRegionsAsync(region0.getEncodedNameAsBytes(), region1.getEncodedNameAsBytes(),
+ true);
+ UTIL.waitFor(timeout, () -> admin.getRegions(tableName).size() == 2);
+ List<RegionInfo> mergedRegions = admin.getRegions(tableName);
+ RegionInfo mergedRegion =
+ mergedRegions.get(0).getEncodedName().equals(region2.getEncodedName())
+ ? mergedRegions.get(1)
+ : mergedRegions.get(0);
+ // snapshot
+ admin.snapshot(snapshotName, tableName);
+ Assert.assertEquals(1, admin.listSnapshots().size());
+ // major compact
+ admin.majorCompactRegion(mergedRegion.getRegionName());
+ // wait until merged region has no reference
+ UTIL.waitFor(timeout, () -> {
+ try {
+ for (RegionServerThread regionServerThread : UTIL.getMiniHBaseCluster()
+ .getRegionServerThreads()) {
+ HRegionServer regionServer = regionServerThread.getRegionServer();
+ for (HRegion subRegion : regionServer.getRegions(tableName)) {
+ if (subRegion.getRegionInfo().getEncodedName()
+ .equals(mergedRegion.getEncodedName())) {
+ regionServer.getCompactedHFilesDischarger().chore();
+ }
+ }
+ }
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ HRegionFileSystem regionFs = HRegionFileSystem
+ .openRegionFromFileSystem(UTIL.getConfiguration(), fs, tableDir, mergedRegion, true);
+ return !regionFs.hasReferences(admin.getDescriptor(tableName));
+ } catch (IOException e) {
+ LOG.warn("Failed check merged region has no reference", e);
+ return false;
+ }
+ });
+ // run catalog janitor to clean and wait for parent regions are archived
+ UTIL.getMiniHBaseCluster().getMaster().getCatalogJanitor().choreForTesting();
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (FileStatus fileStatus : fs.listStatus(tableDir)) {
+ String name = fileStatus.getPath().getName();
+ if (name.equals(region0.getEncodedName()) || name.equals(region1.getEncodedName())) {
+ return false;
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Check if parent regions are archived error", e);
+ return false;
+ }
+ });
+ // set file modify time and then run cleaner
+ long time = System.currentTimeMillis() - TimeToLiveHFileCleaner.DEFAULT_TTL * 1000;
+ traverseAndSetFileTime(HFileArchiveUtil.getArchivePath(conf), time);
+ UTIL.getMiniHBaseCluster().getMaster().getHFileCleaner().runCleaner();
+ // scan snapshot
+ try (TableSnapshotScanner scanner = new TableSnapshotScanner(conf,
+ UTIL.getDataTestDirOnTestFS(snapshotName), snapshotName, new Scan(bbb, yyy))) {
+ verifyScanner(scanner, bbb, yyy);
+ }
+ } catch (Exception e) {
+ LOG.error("scan snapshot error", e);
+ Assert.fail("Should not throw FileNotFoundException");
Review comment:
Don't you really want to make sure just the `TableSnapshotScanner` doesnt' throw a `FileNotFoundException`? Like, maybe lift this catch up into that try block specifically?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] karthikhw commented on issue #1230: Backport HBASE-23553 to
branch-2.1
Posted by GitBox <gi...@apache.org>.
karthikhw commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593675796
Sorry @joshelser. Its my bad! I simply back-ported theses changes, just only excluding compactionSwitch in the code. I should have done review before committing. Pls review the latest commit.
||Are you sure the test still has coverage for the issue in HBASE-23553?
Yes @busbey This test covers HBASE-23553.
Usually, we don't need compactionSwitch in this test case, as we know it never triggers compaction since it only loads less data and an individual test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] busbey commented on issue #1230: Backport HBASE-23553 to
branch-2.1
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593499978
note for committers if I'm not the one who pushes this, the following commit message should be used:
```
HBASE-23553 Snapshot referenced data files are deleted in some case
Backport to branch-2.1 via HBASE-23915 by Karthik P. Differs from
original by skipping test-only need for the "turn compaction on/off"
feature.
Closes #1230
Co-authored-by: Karthik Palanisamy <kp...@hortonworks.com>
```
Karthik I used the email address associated with your github account above. let us know if you'd prefer a different one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1230: Backport HBASE-23553
to branch-2.1
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593767885
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 28s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-2.1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 30s | branch-2.1 passed |
| +1 :green_heart: | compile | 1m 13s | branch-2.1 passed |
| +1 :green_heart: | checkstyle | 1m 41s | branch-2.1 passed |
| +1 :green_heart: | shadedjars | 5m 1s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | branch-2.1 passed |
| +0 :ok: | spotbugs | 3m 29s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 27s | branch-2.1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 20s | the patch passed |
| +1 :green_heart: | compile | 1m 16s | the patch passed |
| +1 :green_heart: | javac | 1m 16s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 42s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 27m 4s | Patch does not cause any errors with Hadoop 2.7.7 2.8.5 or 3.0.3 3.1.2. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
| +1 :green_heart: | findbugs | 3m 28s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 250m 37s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. |
| | | 326m 55s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.replication.TestReplicationDisableInactivePeer |
| | hadoop.hbase.replication.TestReplicationSmallTests |
| | hadoop.hbase.replication.TestReplicationKillSlaveRS |
| | hadoop.hbase.replication.TestReplicationKillSlaveRSWithSeparateOldWALs |
| | hadoop.hbase.replication.regionserver.TestRegionReplicaReplicationEndpoint |
| | hadoop.hbase.regionserver.TestSplitTransactionOnCluster |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1230 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 047a8be2e5b1 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1230/out/precommit/personality/provided.sh |
| git revision | branch-2.1 / 5b3e59e234 |
| Default Java | 1.8.0_181 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/2/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/2/testReport/ |
| Max. process+thread count | 4805 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386503431
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -303,4 +312,131 @@ private static void verifyRow(Result result) throws IOException {
}
}
+ @Test
+ public void testMergeRegion() throws Exception {
+ setupCluster();
+ TableName tableName = TableName.valueOf("testMergeRegion");
+ String snapshotName = tableName.getNameAsString() + "_snapshot";
+ Configuration conf = UTIL.getConfiguration();
+ Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir();
+ long timeout = 20000; // 20s
+ try (Admin admin = UTIL.getAdmin()) {
+ List<String> serverList = admin.getRegionServers().stream().map(sn -> sn.getServerName())
+ .collect(Collectors.toList());
+ // create table with 3 regions
+ Table table = UTIL.createTable(tableName, FAMILIES, 1, bbb, yyy, 3);
+ List<RegionInfo> regions = admin.getRegions(tableName);
+ Assert.assertEquals(3, regions.size());
+ RegionInfo region0 = regions.get(0);
+ RegionInfo region1 = regions.get(1);
+ RegionInfo region2 = regions.get(2);
+ // put some data in the table
+ UTIL.loadTable(table, FAMILIES);
+ admin.flush(tableName);
+ // wait flush is finished
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (RegionInfo region : regions) {
+ Path regionDir = new Path(tableDir, region.getEncodedName());
+ for (Path familyDir : FSUtils.getFamilyDirs(fs, regionDir)) {
+ if (fs.listStatus(familyDir).length != 1) {
+ return false;
+ }
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Failed check if flush is finished", e);
+ return false;
+ }
+ });
+ // merge 2 regions
+ admin.mergeRegionsAsync(region0.getEncodedNameAsBytes(), region1.getEncodedNameAsBytes(),
+ true);
+ UTIL.waitFor(timeout, () -> admin.getRegions(tableName).size() == 2);
+ List<RegionInfo> mergedRegions = admin.getRegions(tableName);
+ RegionInfo mergedRegion =
+ mergedRegions.get(0).getEncodedName().equals(region2.getEncodedName())
+ ? mergedRegions.get(1)
+ : mergedRegions.get(0);
+ // snapshot
+ admin.snapshot(snapshotName, tableName);
+ Assert.assertEquals(1, admin.listSnapshots().size());
+ // major compact
+ admin.majorCompactRegion(mergedRegion.getRegionName());
+ // wait until merged region has no reference
+ UTIL.waitFor(timeout, () -> {
+ try {
+ for (RegionServerThread regionServerThread : UTIL.getMiniHBaseCluster()
+ .getRegionServerThreads()) {
+ HRegionServer regionServer = regionServerThread.getRegionServer();
+ for (HRegion subRegion : regionServer.getRegions(tableName)) {
+ if (subRegion.getRegionInfo().getEncodedName()
+ .equals(mergedRegion.getEncodedName())) {
+ regionServer.getCompactedHFilesDischarger().chore();
+ }
+ }
+ }
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ HRegionFileSystem regionFs = HRegionFileSystem
+ .openRegionFromFileSystem(UTIL.getConfiguration(), fs, tableDir, mergedRegion, true);
+ return !regionFs.hasReferences(admin.getDescriptor(tableName));
+ } catch (IOException e) {
+ LOG.warn("Failed check merged region has no reference", e);
+ return false;
+ }
+ });
+ // run catalog janitor to clean and wait for parent regions are archived
+ UTIL.getMiniHBaseCluster().getMaster().getCatalogJanitor().choreForTesting();
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (FileStatus fileStatus : fs.listStatus(tableDir)) {
+ String name = fileStatus.getPath().getName();
+ if (name.equals(region0.getEncodedName()) || name.equals(region1.getEncodedName())) {
+ return false;
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Check if parent regions are archived error", e);
+ return false;
+ }
+ });
+ // set file modify time and then run cleaner
+ long time = System.currentTimeMillis() - TimeToLiveHFileCleaner.DEFAULT_TTL * 1000;
Review comment:
Can we pull the actual TTL out of the configuration instead of relying on this default ttl variable?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386504245
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -303,4 +312,131 @@ private static void verifyRow(Result result) throws IOException {
}
}
+ @Test
+ public void testMergeRegion() throws Exception {
+ setupCluster();
+ TableName tableName = TableName.valueOf("testMergeRegion");
+ String snapshotName = tableName.getNameAsString() + "_snapshot";
+ Configuration conf = UTIL.getConfiguration();
+ Path rootDir = UTIL.getHBaseCluster().getMaster().getMasterFileSystem().getRootDir();
+ long timeout = 20000; // 20s
+ try (Admin admin = UTIL.getAdmin()) {
+ List<String> serverList = admin.getRegionServers().stream().map(sn -> sn.getServerName())
+ .collect(Collectors.toList());
+ // create table with 3 regions
+ Table table = UTIL.createTable(tableName, FAMILIES, 1, bbb, yyy, 3);
+ List<RegionInfo> regions = admin.getRegions(tableName);
+ Assert.assertEquals(3, regions.size());
+ RegionInfo region0 = regions.get(0);
+ RegionInfo region1 = regions.get(1);
+ RegionInfo region2 = regions.get(2);
+ // put some data in the table
+ UTIL.loadTable(table, FAMILIES);
+ admin.flush(tableName);
+ // wait flush is finished
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (RegionInfo region : regions) {
+ Path regionDir = new Path(tableDir, region.getEncodedName());
+ for (Path familyDir : FSUtils.getFamilyDirs(fs, regionDir)) {
+ if (fs.listStatus(familyDir).length != 1) {
+ return false;
+ }
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Failed check if flush is finished", e);
+ return false;
+ }
+ });
+ // merge 2 regions
+ admin.mergeRegionsAsync(region0.getEncodedNameAsBytes(), region1.getEncodedNameAsBytes(),
+ true);
+ UTIL.waitFor(timeout, () -> admin.getRegions(tableName).size() == 2);
+ List<RegionInfo> mergedRegions = admin.getRegions(tableName);
+ RegionInfo mergedRegion =
+ mergedRegions.get(0).getEncodedName().equals(region2.getEncodedName())
+ ? mergedRegions.get(1)
+ : mergedRegions.get(0);
+ // snapshot
+ admin.snapshot(snapshotName, tableName);
+ Assert.assertEquals(1, admin.listSnapshots().size());
+ // major compact
+ admin.majorCompactRegion(mergedRegion.getRegionName());
+ // wait until merged region has no reference
+ UTIL.waitFor(timeout, () -> {
+ try {
+ for (RegionServerThread regionServerThread : UTIL.getMiniHBaseCluster()
+ .getRegionServerThreads()) {
+ HRegionServer regionServer = regionServerThread.getRegionServer();
+ for (HRegion subRegion : regionServer.getRegions(tableName)) {
+ if (subRegion.getRegionInfo().getEncodedName()
+ .equals(mergedRegion.getEncodedName())) {
+ regionServer.getCompactedHFilesDischarger().chore();
+ }
+ }
+ }
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ HRegionFileSystem regionFs = HRegionFileSystem
+ .openRegionFromFileSystem(UTIL.getConfiguration(), fs, tableDir, mergedRegion, true);
+ return !regionFs.hasReferences(admin.getDescriptor(tableName));
+ } catch (IOException e) {
+ LOG.warn("Failed check merged region has no reference", e);
+ return false;
+ }
+ });
+ // run catalog janitor to clean and wait for parent regions are archived
+ UTIL.getMiniHBaseCluster().getMaster().getCatalogJanitor().choreForTesting();
+ UTIL.waitFor(timeout, () -> {
+ try {
+ Path tableDir = FSUtils.getTableDir(rootDir, tableName);
+ for (FileStatus fileStatus : fs.listStatus(tableDir)) {
+ String name = fileStatus.getPath().getName();
+ if (name.equals(region0.getEncodedName()) || name.equals(region1.getEncodedName())) {
+ return false;
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ LOG.warn("Check if parent regions are archived error", e);
+ return false;
+ }
+ });
+ // set file modify time and then run cleaner
+ long time = System.currentTimeMillis() - TimeToLiveHFileCleaner.DEFAULT_TTL * 1000;
+ traverseAndSetFileTime(HFileArchiveUtil.getArchivePath(conf), time);
+ UTIL.getMiniHBaseCluster().getMaster().getHFileCleaner().runCleaner();
+ // scan snapshot
+ try (TableSnapshotScanner scanner = new TableSnapshotScanner(conf,
+ UTIL.getDataTestDirOnTestFS(snapshotName), snapshotName, new Scan(bbb, yyy))) {
+ verifyScanner(scanner, bbb, yyy);
+ }
+ } catch (Exception e) {
+ LOG.error("scan snapshot error", e);
+ Assert.fail("Should not throw FileNotFoundException");
+ Assert.assertTrue(e.getCause() != null);
Review comment:
Dead code here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on a change in pull request #1230:
Backport HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#discussion_r386727193
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableSnapshotScanner.java
##########
@@ -405,19 +404,23 @@ public void testMergeRegion() throws Exception {
}
});
// set file modify time and then run cleaner
- long time = System.currentTimeMillis() - TimeToLiveHFileCleaner.DEFAULT_TTL * 1000;
+ long cleanerTtl = conf.getLong("hbase.master.hfilecleaner.ttl",
+ TimeToLiveHFileCleaner.DEFAULT_TTL);
+ long time = System.currentTimeMillis() - cleanerTtl * 1000;
traverseAndSetFileTime(HFileArchiveUtil.getArchivePath(conf), time);
UTIL.getMiniHBaseCluster().getMaster().getHFileCleaner().runCleaner();
// scan snapshot
try (TableSnapshotScanner scanner = new TableSnapshotScanner(conf,
UTIL.getDataTestDirOnTestFS(snapshotName), snapshotName, new Scan(bbb, yyy))) {
verifyScanner(scanner, bbb, yyy);
+ } catch (IOException e) {
+ Assert.assertTrue(e.getCause() != null);
Review comment:
I don't think you need to have this catch for `IOException` nor the outer catch for `Exception`. If the exception propagates up, it will cause the test case to fail :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] karthikhw edited a comment on issue #1230: Backport
HBASE-23553 to branch-2.1
Posted by GitBox <gi...@apache.org>.
karthikhw edited a comment on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593675796
Sorry @joshelser. Its my bad! I simply back-ported theses changes, just only excluding compactionSwitch in the code. I should have done review before committing. Pls review the latest commit.
||Are you sure the test still has coverage for the issue in HBASE-23553?
Yes @busbey This test covers HBASE-23553.
Usually, we don't need compactionSwitch in this test case, as we know it never triggers compaction since it only loads less data with no manual compaction and only an individual test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] busbey commented on issue #1230: Backport HBASE-23553 to
branch-2.1
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593153545
Are you sure the test still has coverage for the issue in HBASE-23553?
If I apply the changes locally the test passes with and without the relevant changes to `SnapshotReferenceUtil`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1230: Backport HBASE-23553
to branch-2.1
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1230: Backport HBASE-23553 to branch-2.1
URL: https://github.com/apache/hbase/pull/1230#issuecomment-593105737
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 47s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-2.1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 45s | branch-2.1 passed |
| +1 :green_heart: | compile | 0m 56s | branch-2.1 passed |
| +1 :green_heart: | checkstyle | 1m 34s | branch-2.1 passed |
| +1 :green_heart: | shadedjars | 4m 22s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | branch-2.1 passed |
| +0 :ok: | spotbugs | 3m 5s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 3s | branch-2.1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 22m 23s | Patch does not cause any errors with Hadoop 2.7.7 2.8.5 or 3.0.3 3.1.2. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
| +1 :green_heart: | findbugs | 3m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 223m 41s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 291m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1230 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 0c613bd371f9 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1230/out/precommit/personality/provided.sh |
| git revision | branch-2.1 / e15525fcec |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/1/testReport/ |
| Max. process+thread count | 4543 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1230/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services