[GitHub] [dubbo] DiegoKrupitza commented on a change in pull request #8928: [3.0] destroy executor and improve tests 0926

[GitBox <gi...@apache.org>]

DiegoKrupitza commented on a change in pull request #8928:
URL: https://github.com/apache/dubbo/pull/8928#discussion_r716240615



##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java
##########
@@ -271,14 +272,23 @@ public void loop(boolean notify) {
         if (firstBuildCache.compareAndSet(true,false)) {
             buildCache(notify);
         }
-        if (notify) {
-            if (loopPermitNotify.tryAcquire()) {
-                loopPool.submit(new NotifyLoopRunnable(true, loopPermitNotify));
+
+        try {
+            if (notify) {

Review comment:
       Nested `if`s make the code unreadable. Here would be better to merge the nested `if`s into a single `if`.
   The same applies to the `if` in the `else` statement.
   
   I would suggest:
   ```
   if( notify && loopPermitNotify.tryAcquire() ) {
     loopPool.submit(new NotifyLoopRunnable(true, loopPermitNotify));
   }else if(loopPermit.tryAcquire()) {
     loopPool.submit(new NotifyLoopRunnable(false, loopPermit));
   }
   ```




-- 
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: notifications-unsubscribe@dubbo.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org