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-25475: Unset zk based wal splitting explicitly in tests #2891

Merged
merged 1 commit into from
Jan 17, 2021
Merged

HBASE-25475: Unset zk based wal splitting explicitly in tests #2891

merged 1 commit into from
Jan 17, 2021

Conversation

dasanjan1296
Copy link
Contributor

@dasanjan1296 dasanjan1296 commented Jan 16, 2021 •
edited
Loading

  • zk based wal splitting was deprecated as part of HBASE-24632 and the changes still aren't there in branch-2.3. Hence the assumption of default wal splitting to be procedure-based rather than zk-based doesn't hold for the said branch. The same needs to be considered while setting up the tests in TestSplitWALManager in absence of which the tests were failing in the branch due to NPE.
  • The changes have been tested on branch-2.3 and the test failures are also gone.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 35s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+1 💚 mvninstall 3m 52s master passed
+1 💚 compile 3m 21s master passed
+1 💚 checkstyle 1m 7s master passed
+1 💚 spotbugs 2m 5s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 40s the patch passed
+1 💚 compile 3m 14s the patch passed
+1 💚 javac 3m 14s the patch passed
+1 💚 checkstyle 1m 2s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 17m 39s Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.
+1 💚 spotbugs 2m 13s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 16s The patch does not generate ASF License warnings.
46m 44s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #2891
JIRA Issue HBASE-25475
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux 053032b6e84e 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 643548f
Default Java AdoptOpenJDK-1.8.0_232-b09
Max. process+thread count 96 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/console
versions git=2.17.1 maven=3.6.3 spotbugs=3.1.12
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@virajjasani
Copy link
Contributor

@dasanjan1296 Since HBASE-24632 has fixVersion: 3.0.0 and 2.4.0, this Jira is applicable to releases 2.4.0+ and 3.0.0+ only. Hence, we cannot make this change in branch-2.3. I believe we might have to remove the test.

FYI @saintstack @wchevreuil any other recommendations?

@virajjasani
Copy link
Contributor

Although this change can be merged on master, branch-2 and branch-2.4. But as far as issue with recent test on branch-2.3 is concerned, I think we cannot backport this PR change to branch-2.3.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 32s Docker mode activated.
-0 ⚠️ 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 💚 mvninstall 4m 1s master passed
+1 💚 compile 0m 58s master passed
+1 💚 shadedjars 6m 41s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 39s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 34s the patch passed
+1 💚 compile 0m 58s the patch passed
+1 💚 javac 0m 58s the patch passed
+1 💚 shadedjars 6m 32s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 37s the patch passed
_ Other Tests _
+1 💚 unit 143m 4s hbase-server in the patch passed.
169m 52s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #2891
JIRA Issue HBASE-25475
Optional Tests javac javadoc unit shadedjars compile
uname Linux 5ba156c95d66 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 643548f
Default Java AdoptOpenJDK-1.8.0_232-b09
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/testReport/
Max. process+thread count 4720 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@dasanjan1296
Copy link
Contributor Author

dasanjan1296 commented Jan 16, 2021 •
edited
Loading

@virajjasani Please correct me if I'm wrong. As part of HBASE-21588, procedure-based WAL splitting was introduced and was also backported to branch-2.3. However, as part of HBASE-24632, procedure-based WAL splitting was made as default(on 3.0.0 and 2.4.0) which is why this change is required especially on the other branches(including branch-2.3) so that we explicitly use procedure-based WAL splitting in the tests. I made an incorrect assumption while writing the test that procedure-based splitting is the default way for all branches.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 12s Docker mode activated.
-0 ⚠️ 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 💚 mvninstall 4m 53s master passed
+1 💚 compile 1m 12s master passed
+1 💚 shadedjars 7m 19s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 44s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 4m 32s the patch passed
+1 💚 compile 1m 13s the patch passed
+1 💚 javac 1m 13s the patch passed
+1 💚 shadedjars 7m 19s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 42s the patch passed
_ Other Tests _
+1 💚 unit 199m 53s hbase-server in the patch passed.
230m 55s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #2891
JIRA Issue HBASE-25475
Optional Tests javac javadoc unit shadedjars compile
uname Linux 041ec2f79f3d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 643548f
Default Java AdoptOpenJDK-11.0.6+10
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/testReport/
Max. process+thread count 3343 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2891/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@virajjasani
Copy link
Contributor

@virajjasani Please correct me if I'm wrong. As part of HBASE-21588, procedure-based WAL splitting was introduced and was also backported to branch-2.3. However, as part of HBASE-24632, procedure-based WAL splitting was made as default(on 3.0.0 and 2.4.0) which is why this change is required especially on the other branches(including branch-2.3) so that we explicitly use procedure-based WAL splitting in the tests. I made an incorrect assumption while writing the test that procedure-based splitting is the default way for all branches.

@dasanjan1296 your understanding of feature rollout is correct. However, since master and branch-2, 2.4 are the ones with proc based WAL splitting as default, current PR changes should be applied to them. For branch-2.3, ZK based WAL splitting is default but proc based WAL split option is available I believe. Hence, for branch-2.3 test, rather than changing current test file, you can create new one and disable ZK based WAL splitting prop specifically there. That way we will be able to write test only in that new test class without affecting existing one because existing one behaves with default behaviour.

@dasanjan1296
Copy link
Contributor Author

dasanjan1296 commented Jan 17, 2021 •
edited
Loading

@virajjasani Thanks a lot for the clarification! However, I still have one concern. In the test file, viz. TestSplitWALManager.java, we are already disabling zk-based WAL splitting[here] as part of the common mini-cluster setup in the Test Fixtures.
The change in this PR is to disable zk-based splitting only in one Test Case as it requires spawning another mini-cluster. This change shouldn't affect the other tests in the test file and is also in line with the test fixtures.

@virajjasani
Copy link
Contributor

Oh I see. Makes sense.

virajjasani
virajjasani approved these changes Jan 17, 2021
View reviewed changes
Copy link
Contributor

@virajjasani virajjasani left a comment

Choose a reason for hiding this comment

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

+1

@virajjasani virajjasani merged commit d5debe1 into apache:master Jan 17, 2021
virajjasani pushed a commit that referenced this pull request Jan 17, 2021
@dasanjan1296 @virajjasani
HBASE-25475 : Unset zk based wal splitting explicitly in tests (ADDEN…
fbc4478 
…DUM) (#2891)

Signed-off-by: Viraj Jasani <vjasani@apache.org>
virajjasani pushed a commit that referenced this pull request Jan 17, 2021
@dasanjan1296 @virajjasani
HBASE-25475 : Unset zk based wal splitting explicitly in tests (ADDEN…
7aca781 
…DUM) (#2891)

Signed-off-by: Viraj Jasani <vjasani@apache.org>
virajjasani pushed a commit that referenced this pull request Jan 17, 2021
@dasanjan1296 @virajjasani
HBASE-25475 : Unset zk based wal splitting explicitly in tests (ADDEN…
5058cd9 
…DUM) (#2891)

Signed-off-by: Viraj Jasani <vjasani@apache.org>
virajjasani pushed a commit that referenced this pull request Jan 17, 2021
@dasanjan1296 @virajjasani
HBASE-25475 : Unset zk based wal splitting explicitly in tests (ADDEN…
d230940 
…DUM) (#2891)

Signed-off-by: Viraj Jasani <vjasani@apache.org>
wchevreuil pushed a commit to wchevreuil/hbase that referenced this pull request May 24, 2021
@dasanjan1296 @virajjasani
HBASE-25475 : Unset zk based wal splitting explicitly in tests (ADDEN…
ac614a8 
…DUM) (apache#2891)

Signed-off-by: Viraj Jasani <vjasani@apache.org>
(cherry picked from commit d230940)
Change-Id: Iddeafed66e35e6f98951e973457c957b35349d8d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@virajjasani virajjasani virajjasani approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dasanjan1296 @Apache-HBase @virajjasani