You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/12/16 01:39:40 UTC

[GitHub] [rocketmq] xiaoyifang opened a new issue, #5710: [code issue] the use of CountDownLatch2

xiaoyifang opened a new issue, #5710:
URL: https://github.com/apache/rocketmq/issues/5710

   background:
   the CountDownLatch2 is only used in ServiceThread .
   it is a reimplementation of CountDownLatch and offered a new method reset.
   
   I think the CountDownLatch2 can be replace with java `Condition` class for the following reasons:
   1, CountDownLatch does not offer reset ,as it was designed according to the javadoc ,
   ![image](https://user-images.githubusercontent.com/105986/208000791-4cdd1ed8-9037-4bce-9e64-ad03594ba32a.png)
   2, the instruction order in the ServiceThread is indefinit in  some worst cases.
   the code is in the ServiceThread. and the reset in used in `waitForRuning`.
   https://github.com/apache/rocketmq/blob/aa7a442505ac012d1bc61b89bf10c41646a15005/common/src/main/java/org/apache/rocketmq/common/ServiceThread.java#L129-L141
   
   in multithread environment.
   ![image](https://user-images.githubusercontent.com/105986/208002112-cad2d8e5-6a9d-44cd-a9e3-98108191e9d7.png)
   
   the execution order will have different result ,and in the worst cases ,  `waitForRunning` will have to wait some extra `interval` more millionseconds.
   
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org.apache.org

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


Re: [I] [code issue] the use of CountDownLatch2 [rocketmq]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #5710:
URL: https://github.com/apache/rocketmq/issues/5710#issuecomment-1867071070

   This issue is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] xiaoyifang commented on issue #5710: [code issue] the use of CountDownLatch2

Posted by GitBox <gi...@apache.org>.
xiaoyifang commented on issue #5710:
URL: https://github.com/apache/rocketmq/issues/5710#issuecomment-1354252013

   I'll submit a pr later


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] drpmma commented on issue #5710: [code issue] the use of CountDownLatch2

Posted by GitBox <gi...@apache.org>.
drpmma commented on issue #5710:
URL: https://github.com/apache/rocketmq/issues/5710#issuecomment-1354084653

   > the `waitForRunning`'s behavior in the ServiceThread is indefinite in some worst cases.
   
   There is indeed a concurrent issue on `waitPoint`, but it will not lead to indefinite waiting.
   
   ```java
   protected void waitForRunning(long interval) { 
        if (hasNotified.compareAndSet(true, false)) { 
            this.onWaitEnd(); 
            return; 
        } 
     
        // 1. execute wakeup() here will cause a extra waiting
   
        waitPoint.reset(); 
     
       // 2. execute wakeup() here will lead to immediate wakeup without waiting
   
        try { 
            waitPoint.await(interval, TimeUnit.MILLISECONDS); 
        } catch (InterruptedException e) { 
            log.error("Interrupted", e); 
   ```
   
   As the annotation below, the `wakeup()` invocation before `waitPoint.reset()` will cause a extra waiting while `wakeup()` execution after `waitPoint.reset()` will lead to immediate wakeup without waiting.
   
   There is indeed a concurrent issue and do you have the indentation to submit to pr to fix it?
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

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


Re: [I] [code issue] the use of CountDownLatch2 [rocketmq]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #5710:
URL: https://github.com/apache/rocketmq/issues/5710#issuecomment-1868618239

   This issue was closed because it has been inactive for 3 days since being marked as stale.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

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


Re: [I] [code issue] the use of CountDownLatch2 [rocketmq]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #5710: [code issue] the use of CountDownLatch2
URL: https://github.com/apache/rocketmq/issues/5710


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

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