-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29397 Deadlock in BufferedMuratorOverAsyncBufferedMutator #7107
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request addresses the deadlock issue in BufferedMutatorOverAsyncBufferedMutator and improves test coverage for buffered mutator behavior. Key changes include new multi-threaded tests in TestBufferedMutator.java, modifications to test setup/teardown routines, and refactoring of the asynchronous mutation listener in BufferedMutatorOverAsyncBufferedMutator.java.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestBufferedMutator.java | Added multi-thread tests and improved test lifecycle management to better simulate concurrent mutation scenarios. |
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutatorOverAsyncBufferedMutator.java | Removed the synchronized block in the asynchronous mutation listener; however, key operations (bufferedSize decrement and future completion) appear to be omitted. |
Comments suppressed due to low confidence (2)
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutatorOverAsyncBufferedMutator.java:137
- The removal of the synchronized block also removed critical operations: decrementing bufferedSize, handling errors, and completing the future. These operations are essential to ensuring that mutation processing and internal flush behavior work correctly. Please revert or rework the changes to include these steps.
addListener(fs.get(i), (r, e) -> {
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestBufferedMutator.java:86
- [nitpick] The test method name 'test' is ambiguous. Consider renaming it to a more descriptive name (e.g., testBufferedMutatorSimple) to better reflect its purpose.
@Test public void test() throws Exception {
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
We need to get this in immediately since it the bug affects bunch of tests under hbase-mapreduce... With this patch in place, I've successfully finished a 10B ITBLL run. Please HBASE-27919. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems alright to me. Nice avoidance of synchronized block.
The synchronized block was added recently in HBASE-29394, which is incorrect... Thanks Nick. Let me merge... |
Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> (cherry picked from commit a8a57ad)
No description provided.