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 2023/01/19 14:57:21 UTC
[GitHub] [hbase] bbeaudreault opened a new pull request, #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
bbeaudreault opened a new pull request, #4986:
URL: https://github.com/apache/hbase/pull/4986
There already existed a robust reference check method for cleaning splits, `checkDaughterInFs`. I cleaned that up a little bit and re-use it in cleanMergeRegion. This way both cleaning use the same reference checking logic. This should be easier to maintain going forward.
I added tests in TestCatalogJanitor -- there were none for merges, so i added a base case and a failure mode case.
I decided not to modify HRegionFileSystem.openRegionFromFileSystem -- even if I modified that to throw FileNotFoundException, from a defensive coding perspective I'd rather not assume that exception refers to the region dir specifically. This code is highly destructive, so we rely on an explicit `exists` call and only delete if that successfully returns false within our method.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault merged pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
bbeaudreault merged PR #4986:
URL: https://github.com/apache/hbase/pull/4986
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4986:
URL: https://github.com/apache/hbase/pull/4986#discussion_r1081645332
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java:
##########
@@ -387,59 +390,45 @@ private static boolean hasNoReferences(final Pair<Boolean, Boolean> p) {
}
/**
- * Checks if a daughter region -- either splitA or splitB -- still holds references to parent.
- * @param parent Parent region
- * @param daughter Daughter region
- * @return A pair where the first boolean says whether or not the daughter region directory exists
- * in the filesystem and then the second boolean says whether the daughter has references
- * to the parent.
+ * Checks if a region still holds references to parent.
+ * @param tableName The table for the region
+ * @param region The region to check
+ * @return A pair where the first boolean says whether the region directory exists in the
+ * filesystem and then the second boolean says whether the region has references to a
+ * parent.
*/
- private static Pair<Boolean, Boolean> checkDaughterInFs(MasterServices services,
- final RegionInfo parent, final RegionInfo daughter) throws IOException {
- if (daughter == null) {
+ private static Pair<Boolean, Boolean> checkRegionReferences(MasterServices services,
+ TableName tableName, RegionInfo region) throws IOException {
+ if (region == null) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
FileSystem fs = services.getMasterFileSystem().getFileSystem();
Path rootdir = services.getMasterFileSystem().getRootDir();
- Path tabledir = CommonFSUtils.getTableDir(rootdir, daughter.getTable());
-
- Path daughterRegionDir = new Path(tabledir, daughter.getEncodedName());
-
- HRegionFileSystem regionFs;
+ Path tabledir = CommonFSUtils.getTableDir(rootdir, tableName);
+ Path regionDir = new Path(tabledir, region.getEncodedName());
try {
- if (!CommonFSUtils.isExists(fs, daughterRegionDir)) {
+ if (!CommonFSUtils.isExists(fs, regionDir)) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
} catch (IOException ioe) {
- LOG.error("Error trying to determine if daughter region exists, "
- + "assuming exists and has references", ioe);
+ LOG.error("Error trying to determine if region exists, assuming exists and has references",
+ ioe);
return new Pair<>(Boolean.TRUE, Boolean.TRUE);
Review Comment:
Correct. Previously we'd treat this error as "lets assume it doesn't exist and go forward with cleanup". This assumption was inaccurate in at least some failure modes, so unsafe. The new assumption is "we don't know if the region exists, so let's not risk cleaning up". This is safer, but the downside is we might wait longer to GC merged regions. I think that's not a problem at all.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1398437068
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 11s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 33s | master passed |
| +1 :green_heart: | compile | 2m 22s | master passed |
| +1 :green_heart: | checkstyle | 0m 37s | master passed |
| +1 :green_heart: | spotless | 0m 43s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 30s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | the patch passed |
| +1 :green_heart: | compile | 2m 22s | the patch passed |
| +1 :green_heart: | javac | 2m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 36s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 13s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 41s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 34s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 38m 38s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 8e4b51fd8cd9 5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 82 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1397182983
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 24s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | master passed |
| +1 :green_heart: | compile | 2m 26s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 22s | the patch passed |
| +1 :green_heart: | compile | 2m 23s | the patch passed |
| +1 :green_heart: | javac | 2m 23s | the patch passed |
| -0 :warning: | checkstyle | 0m 32s | hbase-server: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 58s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 31m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux faea28bbf26b 5.4.0-1093-aws #102~18.04.2-Ubuntu SMP Wed Dec 7 00:31:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / ad8f28e297 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1397532353
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 52s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 36s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 4m 32s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 2s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 0s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 273m 11s | hbase-server in the patch failed. |
| | | 294m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8cbf629f8b2a 5.4.0-1088-aws #96~18.04.1-Ubuntu SMP Mon Oct 17 02:57:48 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / ad8f28e297 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/testReport/ |
| Max. process+thread count | 2623 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a diff in pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
virajjasani commented on code in PR #4986:
URL: https://github.com/apache/hbase/pull/4986#discussion_r1081618087
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java:
##########
@@ -387,59 +390,45 @@ private static boolean hasNoReferences(final Pair<Boolean, Boolean> p) {
}
/**
- * Checks if a daughter region -- either splitA or splitB -- still holds references to parent.
- * @param parent Parent region
- * @param daughter Daughter region
- * @return A pair where the first boolean says whether or not the daughter region directory exists
- * in the filesystem and then the second boolean says whether the daughter has references
- * to the parent.
+ * Checks if a region still holds references to parent.
+ * @param tableName The table for the region
+ * @param region The region to check
+ * @return A pair where the first boolean says whether the region directory exists in the
+ * filesystem and then the second boolean says whether the region has references to a
+ * parent.
*/
- private static Pair<Boolean, Boolean> checkDaughterInFs(MasterServices services,
- final RegionInfo parent, final RegionInfo daughter) throws IOException {
- if (daughter == null) {
+ private static Pair<Boolean, Boolean> checkRegionReferences(MasterServices services,
+ TableName tableName, RegionInfo region) throws IOException {
+ if (region == null) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
FileSystem fs = services.getMasterFileSystem().getFileSystem();
Path rootdir = services.getMasterFileSystem().getRootDir();
- Path tabledir = CommonFSUtils.getTableDir(rootdir, daughter.getTable());
-
- Path daughterRegionDir = new Path(tabledir, daughter.getEncodedName());
-
- HRegionFileSystem regionFs;
+ Path tabledir = CommonFSUtils.getTableDir(rootdir, tableName);
+ Path regionDir = new Path(tabledir, region.getEncodedName());
try {
- if (!CommonFSUtils.isExists(fs, daughterRegionDir)) {
+ if (!CommonFSUtils.isExists(fs, regionDir)) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
} catch (IOException ioe) {
- LOG.error("Error trying to determine if daughter region exists, "
- + "assuming exists and has references", ioe);
+ LOG.error("Error trying to determine if region exists, assuming exists and has references",
+ ioe);
return new Pair<>(Boolean.TRUE, Boolean.TRUE);
Review Comment:
So this is the big difference b/ the two implementations right?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1397469665
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 53s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 35s | the patch passed |
| +1 :green_heart: | compile | 0m 48s | the patch passed |
| +1 :green_heart: | javac | 0m 48s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 48s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 213m 59s | hbase-server in the patch passed. |
| | | 235m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6fb9447ef100 5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / ad8f28e297 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/testReport/ |
| Max. process+thread count | 2493 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1398700220
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 7s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 14s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 4m 24s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 15s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 20s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 213m 49s | hbase-server in the patch passed. |
| | | 236m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0c5a67b10f0f 5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/testReport/ |
| Max. process+thread count | 2715 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1398352078
Thanks all for the review.
Having trouble with some flaky tests. I'm going to let it run once more to see if we can get a clean build. So far I've run the failing tests manually and they succeed locally.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1398686944
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 0m 35s | master passed |
| +1 :green_heart: | shadedjars | 4m 31s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 31s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 206m 31s | hbase-server in the patch passed. |
| | | 227m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8369092bfc1a 5.4.0-1088-aws #96~18.04.1-Ubuntu SMP Mon Oct 17 02:57:48 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/testReport/ |
| Max. process+thread count | 2818 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4986:
URL: https://github.com/apache/hbase/pull/4986#issuecomment-1397705029
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 51s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 34s | master passed |
| +1 :green_heart: | compile | 2m 46s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 47s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 50s | the patch passed |
| +1 :green_heart: | compile | 2m 42s | the patch passed |
| +1 :green_heart: | javac | 2m 42s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 43s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 1s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 42s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 6s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 37m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4986 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux f85311caf618 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / ad8f28e297 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4986/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a diff in pull request #4986: HBASE-27579 CatalogJanitor can cause data loss due to errors during cleanMergeRegion
Posted by GitBox <gi...@apache.org>.
virajjasani commented on code in PR #4986:
URL: https://github.com/apache/hbase/pull/4986#discussion_r1082136488
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java:
##########
@@ -387,59 +390,45 @@ private static boolean hasNoReferences(final Pair<Boolean, Boolean> p) {
}
/**
- * Checks if a daughter region -- either splitA or splitB -- still holds references to parent.
- * @param parent Parent region
- * @param daughter Daughter region
- * @return A pair where the first boolean says whether or not the daughter region directory exists
- * in the filesystem and then the second boolean says whether the daughter has references
- * to the parent.
+ * Checks if a region still holds references to parent.
+ * @param tableName The table for the region
+ * @param region The region to check
+ * @return A pair where the first boolean says whether the region directory exists in the
+ * filesystem and then the second boolean says whether the region has references to a
+ * parent.
*/
- private static Pair<Boolean, Boolean> checkDaughterInFs(MasterServices services,
- final RegionInfo parent, final RegionInfo daughter) throws IOException {
- if (daughter == null) {
+ private static Pair<Boolean, Boolean> checkRegionReferences(MasterServices services,
+ TableName tableName, RegionInfo region) throws IOException {
+ if (region == null) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
FileSystem fs = services.getMasterFileSystem().getFileSystem();
Path rootdir = services.getMasterFileSystem().getRootDir();
- Path tabledir = CommonFSUtils.getTableDir(rootdir, daughter.getTable());
-
- Path daughterRegionDir = new Path(tabledir, daughter.getEncodedName());
-
- HRegionFileSystem regionFs;
+ Path tabledir = CommonFSUtils.getTableDir(rootdir, tableName);
+ Path regionDir = new Path(tabledir, region.getEncodedName());
try {
- if (!CommonFSUtils.isExists(fs, daughterRegionDir)) {
+ if (!CommonFSUtils.isExists(fs, regionDir)) {
return new Pair<>(Boolean.FALSE, Boolean.FALSE);
}
} catch (IOException ioe) {
- LOG.error("Error trying to determine if daughter region exists, "
- + "assuming exists and has references", ioe);
+ LOG.error("Error trying to determine if region exists, assuming exists and has references",
+ ioe);
return new Pair<>(Boolean.TRUE, Boolean.TRUE);
Review Comment:
> This is safer, but the downside is we might wait longer to GC merged regions. I think that's not a problem at all.
+1
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org