You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/02/01 04:10:43 UTC

[GitHub] [ozone] Xushaohong opened a new pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Xushaohong opened a new pull request #1861:
URL: https://github.com/apache/ozone/pull/1861


   ## What changes were proposed in this pull request? 
   Problem: In PipelineManager, the methods of activating and deactivating pipeline methods don't have the lock that originally supposed to have. Thus the result is that activate and activate operation could be requested concurrently and may lead to reading the stale version of the pipeline state. 
   
   Solution: Add the write lock each time before calling the statemanager.
   
   ## What is the link to the Apache JIRA 
   https://issues.apache.org/jira/browse/HDDS-4756 
   
   ## How was this patch tested?
   The locked version is tested based on the tps upper bound test. The result shows that the number of ratis calls per second is declined rapidly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] GlenGeng commented on a change in pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#discussion_r567580719



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineManager.java
##########
@@ -572,10 +577,15 @@ public void activatePipeline(PipelineID pipelineID)
   @Override
   public void deactivatePipeline(PipelineID pipelineID)
       throws IOException {
-    Pipeline.PipelineState state = stateManager.
-            getPipeline(pipelineID).getPipelineState();
-    stateManager.deactivatePipeline(pipelineID);
-    updatePipelineStateInDb(pipelineID, state);
+    lock.writeLock().lock();
+    try {
+      Pipeline.PipelineState state = stateManager.
+              getPipeline(pipelineID).getPipelineState();
+      stateManager.deactivatePipeline(pipelineID);
+      updatePipelineStateInDb(pipelineID, state);
+    }finally {

Review comment:
       ditto

##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineManager.java
##########
@@ -557,10 +557,15 @@ public void triggerPipelineCreation() {
   @Override
   public void activatePipeline(PipelineID pipelineID)
       throws IOException {
-    Pipeline.PipelineState state = stateManager.
-            getPipeline(pipelineID).getPipelineState();
-    stateManager.activatePipeline(pipelineID);
-    updatePipelineStateInDb(pipelineID, state);
+    lock.writeLock().lock();
+    try {
+      Pipeline.PipelineState state = stateManager.
+              getPipeline(pipelineID).getPipelineState();
+      stateManager.activatePipeline(pipelineID);
+      updatePipelineStateInDb(pipelineID, state);
+    }finally {

Review comment:
       Indent, `} finally {`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Xushaohong commented on pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
Xushaohong commented on pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#issuecomment-770519378


   > @Xushaohong which is the final version for [HDDS-4756](https://issues.apache.org/jira/browse/HDDS-4756)? This PR or #1862?
   
   Thanks @amaliujia , I submited two PR, one is for master , and the other is for HDDS-2823. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] Xushaohong closed pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
Xushaohong closed pull request #1861:
URL: https://github.com/apache/ozone/pull/1861


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] amaliujia commented on pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#issuecomment-770321357


   cc @GlenGeng 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] amaliujia commented on a change in pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#discussion_r567593895



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineManager.java
##########
@@ -557,10 +557,15 @@ public void triggerPipelineCreation() {
   @Override
   public void activatePipeline(PipelineID pipelineID)
       throws IOException {
-    Pipeline.PipelineState state = stateManager.
-            getPipeline(pipelineID).getPipelineState();
-    stateManager.activatePipeline(pipelineID);
-    updatePipelineStateInDb(pipelineID, state);
+    lock.writeLock().lock();
+    try {
+      Pipeline.PipelineState state = stateManager.
+              getPipeline(pipelineID).getPipelineState();
+      stateManager.activatePipeline(pipelineID);
+      updatePipelineStateInDb(pipelineID, state);
+    }finally {

Review comment:
       +1




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai commented on pull request #1861: HDDS-4756. Add lock for activate/deactivate in SCMPipelineManager

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#issuecomment-772814035


   Thanks @Xushaohong for the fix, @amaliujia @bshashikant and @GlenGeng for the review.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] adoroszlai merged pull request #1861: HDDS-4756. Add lock for activate/deactivate in SCMPipelineManager

Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1861:
URL: https://github.com/apache/ozone/pull/1861


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] amaliujia commented on pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#issuecomment-770613949


   +1. The lock makes sense.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] amaliujia commented on pull request #1861: HDDS-4756. Add lock for activate/deactivate in PipelineManagerV2

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1861:
URL: https://github.com/apache/ozone/pull/1861#issuecomment-770470104


   @Xushaohong which is the final version for HDDS-4756? This PR or https://github.com/apache/ozone/pull/1862?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org