-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDDS-1451 : SCMBlockManager findPipeline and createPipeline are not lock protected. #799
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
|
💔 -1 overall
This message was automatically generated. |
|
/label ozone |
|
/retest |
mukul1987
left a comment
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.
The patch looks good to me. Please change the log lines to add the type and factor.
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
@avijayanhwx the patch looks good to me. Can you please confirm that the test failures are not due to this patch. Thanks in Advance. |
@anuengineer The failures seem unrelated. I have rebased with latest trunk. We can see how this run goes and then commit it. |
|
+1, LGTM. Thanks. |
As per subject, table descriptors moved are - LocalTableDescriptor - RemoteTableDescriptor - HybridTableDescriptor - GuavaCacheTableDescriptor - CachingTableDescriptor Author: Wei Song <[email protected]> Reviewers: Prateek Maheshwari <[email protected]> Closes apache#799 from weisong44/SAMZA-1981
The getPipelines() and createPipeline() already seem to have a lock in their implementation. However, the problem described here involves a race condition between the call to getPipelines and createPipelines in BlockManagerImpl#allocateBlock. The fix is to add another getPipelines check after a failed createPipeline call to get any newly created pipelines.