[GitHub] chickenlj commented on a change in pull request #2498: Optimize resultFuture callback

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

chickenlj commented on a change in pull request #2498: Optimize resultFuture callback
URL: https://github.com/apache/incubator-dubbo/pull/2498#discussion_r218674212
 
 

 ##########
 File path: dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java
 ##########
 @@ -133,7 +133,7 @@ public Object recreate() throws Throwable {
     }
 
     public void thenApplyWithContext(Function<Result, Result> fn) {
-        this.resultFuture = resultFuture.thenApply(fn.compose(beforeContext).andThen(afterContext));
+        resultFuture.thenApply(fn.compose(beforeContext).andThen(afterContext));
 
 Review comment:
   I am not sure if this change can work properly. 
   `this.resultFuture = resultFuture.thenApply(fn.compose(beforeContext).andThen(afterContext));`
   By using the newest Future instance every time, we can guarantee the execute sequences of each Filter' callback. 
   
   For example, if we have a filter chain like this (when result returning back): `Invoker -> Filter1 -> Filter2 -> Filter3`.
   1. If we use the code you provide, when Invoker finishes, it will trigger the callback of Filter1, Filter2 and Filter3;
   2. But if we use the old code, when Invoker finishes, it will trigger callback of Filter1, then if Filter1 callback finishes, it will trigger Filter2, ...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

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