Skip to content

Conversation

@wchevreuil
Copy link
Contributor

…ervers' from master logs, then submit SCPs for each of those.

…ervers' from master logs, then submit SCPs for each of those.
@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 26s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 1m 9s master passed
+1 💚 compile 0m 8s master passed
+1 💚 checkstyle 0m 5s master passed
+1 💚 javadoc 0m 6s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 8s the patch passed
+1 💚 compile 0m 8s the patch passed
+1 💚 javac 0m 8s the patch passed
-1 ❌ checkstyle 0m 3s hbase-tools: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
-1 ❌ whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
+1 💚 unit 2m 4s hbase-tools in the patch passed.
+1 💚 asflicense 0m 5s The patch does not generate ASF License warnings.
5m 34s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/1/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux a435d7ca80ab 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
checkstyle https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/1/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-tools.txt
whitespace https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/1/artifact/yetus-precommit-check/output/whitespace-eol.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/1/testReport/
Max. process+thread count 1223 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/1/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Member

@joshelser joshelser left a comment

Choose a reason for hiding this comment

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

I think my main worry is about relying on log file parsing for this. That's not to say this is flawed -- it just is what it is.

Is there a reason you went for this approach rather than trying to read meta and interrogate the Master as to who the active RegionServers are?

}
}
}
HBCK2 hbck2 = new HBCK2(conf);
Copy link
Member

Choose a reason for hiding this comment

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

What about at dryrun option which can be run first to make sure reasonable servernames are parsed?

@wchevreuil
Copy link
Contributor Author

I think my main worry is about relying on log file parsing for this. That's not to say this is flawed -- it just is what it is.

Yeah, it's fragile indeed, but we are targeting this mainly for the unfortunate souls running hbase versions lower than 2.2.7, 2.3.5 and 2.4.7, as mentioned on the readme, so there shouldn't be any changes on logging format for these already released versions. For any version from the above mentioned onwards, there's already hbck2 recoverUnknown method.

Is there a reason you went for this approach rather than trying to read meta and interrogate the Master as to who the active RegionServers are?

Although meta is already online, master has not completed initialisation (because of namespace table region stuck on a unknown server), so it pushes back any client requests.

@joshelser
Copy link
Member

Although meta is already online, master has not completed initialisation (because of namespace table region stuck on a unknown server), so it pushes back any client requests.

Argh. The gift that keeps on giving.

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 43s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 0m 53s master passed
+1 💚 compile 0m 8s master passed
+1 💚 checkstyle 0m 6s master passed
+1 💚 javadoc 0m 7s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 9s the patch passed
+1 💚 compile 0m 8s the patch passed
+1 💚 javac 0m 8s the patch passed
-1 ❌ checkstyle 0m 4s hbase-tools: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
-1 ❌ unit 2m 17s hbase-tools in the patch failed.
+1 💚 asflicense 0m 6s The patch does not generate ASF License warnings.
4m 54s
Reason Tests
Failed junit tests hbase.TestRegionsMerger
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/2/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux b9155d65bc12 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
checkstyle https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/2/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-tools.txt
unit https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/2/artifact/yetus-precommit-check/output/patch-unit-hbase-tools.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/2/testReport/
Max. process+thread count 1169 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/2/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 52s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 0m 49s master passed
+1 💚 compile 0m 9s master passed
+1 💚 checkstyle 0m 6s master passed
+1 💚 javadoc 0m 7s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 10s the patch passed
+1 💚 compile 0m 9s the patch passed
+1 💚 javac 0m 9s the patch passed
+1 💚 checkstyle 0m 4s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
+1 💚 unit 2m 11s hbase-tools in the patch passed.
+1 💚 asflicense 0m 5s The patch does not generate ASF License warnings.
5m 56s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/3/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux 2d7cf6afb780 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/3/testReport/
Max. process+thread count 1214 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/3/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@wchevreuil wchevreuil requested a review from joshelser May 11, 2021 15:56
Copy link
Contributor

@petersomogyi petersomogyi left a comment

Choose a reason for hiding this comment

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

Looks fine overall. Some nits.

}
} else {
LOG.error("Wrong number of arguments. "
+ "Arguments are: <PATH_TO_MASTER_LOGS> [dryRun]");
Copy link
Contributor

Choose a reason for hiding this comment

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

From the usage message it is not obvious that the [dryRun] parameter should be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to mention the available options here. I guess we can explain the dryRun option in the README only?

Copy link
Contributor

Choose a reason for hiding this comment

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

Having it in the readme is enough but in my opinion, it is still not clear what an operator should use as the second parameter.

hbase org.apache.hbase.RegionsOnUnknownServersRecoverer /var/log/hbase.log true
hbase org.apache.hbase.RegionsOnUnknownServersRecoverer /var/log/hbase.log dryRun

Anything passed that is not true will schedule SCPs because the default option is dryrun=false.

This tool requires the master logs path as parameter. Assuming classpath is properly set, can be run as follows:

```
$ hbase org.apache.hbase.RegionsOnUnknownServersRecoverer PATH_TO_MASTER_LOGS
Copy link
Contributor

Choose a reason for hiding this comment

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

The dry-run option is not mentioned here.

@wchevreuil wchevreuil requested a review from petersomogyi May 12, 2021 12:11
@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 43s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 0m 57s master passed
+1 💚 compile 0m 8s master passed
+1 💚 checkstyle 0m 6s master passed
+1 💚 javadoc 0m 7s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 11s the patch passed
+1 💚 compile 0m 8s the patch passed
+1 💚 javac 0m 8s the patch passed
+1 💚 checkstyle 0m 4s the patch passed
-1 ❌ whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
+1 💚 unit 2m 13s hbase-tools in the patch passed.
+1 💚 asflicense 0m 6s The patch does not generate ASF License warnings.
4m 55s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/4/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux 814902db5e44 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
whitespace https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/4/artifact/yetus-precommit-check/output/whitespace-eol.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/4/testReport/
Max. process+thread count 1214 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/4/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 50s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 0m 53s master passed
+1 💚 compile 0m 9s master passed
+1 💚 checkstyle 0m 6s master passed
+1 💚 javadoc 0m 7s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 9s the patch passed
+1 💚 compile 0m 9s the patch passed
+1 💚 javac 0m 9s the patch passed
+1 💚 checkstyle 0m 3s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
+1 💚 unit 2m 22s hbase-tools in the patch passed.
+1 💚 asflicense 0m 6s The patch does not generate ASF License warnings.
6m 8s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/5/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux 590978ef2ef2 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/5/testReport/
Max. process+thread count 1164 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/5/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 41s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 markdownlint 0m 0s markdownlint was not available.
+0 🆗 spotbugs 0m 0s spotbugs executables are not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 0m 49s master passed
+1 💚 compile 0m 8s master passed
+1 💚 checkstyle 0m 5s master passed
+1 💚 javadoc 0m 7s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 0m 9s the patch passed
+1 💚 compile 0m 8s the patch passed
+1 💚 javac 0m 8s the patch passed
+1 💚 checkstyle 0m 3s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 javadoc 0m 5s the patch passed
_ Other Tests _
+1 💚 unit 2m 14s hbase-tools in the patch passed.
+1 💚 asflicense 0m 5s The patch does not generate ASF License warnings.
4m 40s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/6/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR #86
Optional Tests dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile
uname Linux aadddaaf6640 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux
Build tool maven
git revision master / f118dfb
Default Java Oracle Corporation-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/6/testReport/
Max. process+thread count 1210 (vs. ulimit of 5000)
modules C: hbase-tools U: hbase-tools
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-86/6/console
versions git=2.20.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@wchevreuil wchevreuil merged commit 62b984e into apache:master May 14, 2021
@joshelser
Copy link
Member

Belated +1 from me. Thanks for the dryRun option, Wellington.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants