Skip to content

HBASE-26688 Threads shared EMPTY_RESULT may lead to unexpected client job down - #4072

Closed
YutSean wants to merge 1 commit into
apache:masterfrom
YutSean:HBASE-26688-master
Closed

HBASE-26688 Threads shared EMPTY_RESULT may lead to unexpected client job down#4072
YutSean wants to merge 1 commit into
apache:masterfrom
YutSean:HBASE-26688-master

Conversation

@YutSean

@YutSeanYutSean commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

@Apache-HBase

Apache-HBase commented Jan 27, 2022

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 11sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 1sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 16sMaven dependency ordering for branch
+1 💚mvninstall4m 35smaster passed
+1 💚compile4m 33smaster passed
+1 💚checkstyle1m 44smaster passed
+1 💚spotbugs3m 32smaster passed
-0 ⚠️patch2m 40sUsed diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+0 🆗mvndep0m 14sMaven dependency ordering for patch
+1 💚mvninstall4m 27sthe patch passed
+1 💚compile4m 32sthe patch passed
+1 💚javac4m 32sthe patch passed
+1 💚checkstyle0m 29shbase-client: The patch generated 0 new + 11 unchanged - 2 fixed = 11 total (was 13)
-0 ⚠️checkstyle1m 9shbase-server: The patch generated 1 new + 2 unchanged - 0 fixed = 3 total (was 2)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck22m 11sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs3m 51sthe patch passed
_ Other Tests _
+1 💚asflicense0m 23sThe patch does not generate ASF License warnings.
62m 27s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#4072
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
unameLinux 4dae62b12b3a 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / f6348d4
Default JavaAdoptOpenJDK-1.8.0_282-b08
checkstylehttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
Max. process+thread count86 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache9
Apache9 reviewed Jan 27, 2022
View reviewed changes
Comment threadhbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
}

@Test
public void testAdvanceTwiceOnEmptyCell() throws IOException {

@Apache9Apache9Jan 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Better keep this test but use two assertFalse to confirm that the second advance call will return false?

@YutSeanYutSeanJan 27, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Better keep this test but use two assertFalse to confirm that the second advance call will return false?

After the change, empty list will always return false. I think we could change the UT's name to testAdvanceMultipieOnEmptyCell and use an arbitrary value > 2 to assert false. What do you think?

@Apache9Apache9Jan 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is also fine. Just go ahead.

@YutSeanYutSean changed the title HBASE-26688 Result#advance, current are not thread safe. But the pre-created EMPTY_RESULT like objects can be shared by multi client threadsHBASE-26688 Threads shared EMPTY_RESULT may lead to unexpected client job downJan 27, 2022
@Apache-HBase

Apache-HBase commented Jan 27, 2022

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 6sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for branch
+1 💚mvninstall5m 18smaster passed
+1 💚compile1m 52smaster passed
+1 💚shadedjars9m 6sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 10smaster passed
-0 ⚠️patch10m 37sUsed diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for patch
+1 💚mvninstall5m 5sthe patch passed
+1 💚compile1m 53sthe patch passed
+1 💚javac1m 53sthe patch passed
+1 💚shadedjars9m 9spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 9sthe patch passed
_ Other Tests _
+1 💚unit1m 45shbase-client in the patch passed.
-1 ❌unit208m 5shbase-server in the patch failed.
248m 22s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#4072
Optional Testsjavac javadoc unit shadedjars compile
unameLinux ca28ce3918f0 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / f6348d4
Default JavaAdoptOpenJDK-11.0.10+9
unithttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/testReport/
Max. process+thread count3729 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@YutSeanYutSean closed this Jan 27, 2022
@Apache-HBase

Apache-HBase commented Jan 27, 2022

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 22sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 17sMaven dependency ordering for branch
+1 💚mvninstall5m 5smaster passed
+1 💚compile1m 51smaster passed
+1 💚shadedjars9m 58sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 12smaster passed
-0 ⚠️patch11m 32sUsed diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+0 🆗mvndep0m 19sMaven dependency ordering for patch
+1 💚mvninstall4m 48sthe patch passed
+1 💚compile1m 47sthe patch passed
+1 💚javac1m 47sthe patch passed
+1 💚shadedjars9m 59spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc1m 10sthe patch passed
_ Other Tests _
+1 💚unit1m 48shbase-client in the patch passed.
-1 ❌unit254m 6shbase-server in the patch failed.
297m 46s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#4072
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 873c4c018138 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / f6348d4
Default JavaAdoptOpenJDK-1.8.0_282-b08
unithttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/testReport/
Max. process+thread count2892 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-4072/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

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

Reviewers

@Apache9Apache9Apache9 left review comments

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

@YutSean@Apache-HBase@Apache9