merge async changes in 3.x to 2.7 #3997

chickenlj · 2019-05-08T09:26:48Z

What is the purpose of the change

There's a refactoring work related to async on branch 3.x, it solves most of the problems in milestone 2.7.2 #3287, I think we can simply merge it to master. This can also help us to keep the core concepts in codebase in different branches the same.

Brief changelog

XXXXX

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
If this contribution is large, please follow the Software Donation Guide.

* Result implement CF * Result implement CF * Result implement CF * Add AsyncRpcResult * Fix bugs and refactor Filter * Try to add onSend onError for Filter * invoke different filter method according to result status. * make generic work with async call, including add $invokeAsync * refactor legacy Filter implementation to work with onResponse. * demo changes * Fixes apache#3620, provider attachment lose on consumer side, fix this by reverting RpcContext copy * AsyncRpcResult should always holds an Invocation instance * refactor filter signature * reimplement embedded Filters * use ProviderModel modification in 3.x * Fix address notification processing workflow after merging 3.x branch * Fix UT * Fix UT * Unit test of JValidator; Clean code of JValidator (apache#3723) * Fixes apache#3625 (apache#3730) use constant to replace magic number * Fix conflict when merging master and 3.x * Fix conflict when merging master and 3.x * Result interface itself has Future status. * Fix DefaultFuture UT * Wrap all protocol Invoker with AsyncToSyncInvoker & Fix UT * Add license * fix UT * Fix ut in MonitorFilterTest * avoid duplicate async to sync wrapper * return async result in CacheFilter. * fix UT in CacheFilterTest * Add generic condition check to GenericFilter callback. * Fix UT * Get generic from RpcContext if the value in Invocation is empty. * Fix RSocketProtocol to meet AbstractProtocol adjustment * rename RpcResult to AppResponse to help avoid confusion with AsyncRpcResult. * RSocket module switch to AsyncRpcResult

…AppResponse. (apache#3889) Clear that the `Result` of the call back is actually an `AppResponse`.

…che#3916)

# Conflicts: # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java

chickenlj · 2019-05-08T09:47:17Z

the main PR #3738 merged to 3.x

…fore.

# Conflicts: # dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java # dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java # dubbo-filter/dubbo-filter-cache/src/main/java/org/apache/dubbo/cache/filter/CacheFilter.java # dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorFilter.java # dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/DefaultFuture.java # dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeChannel.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ActiveLimitFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/CompatibleFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ConsumerContextFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ContextFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/EchoFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ExceptionFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ExecuteLimitFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericImplFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/TimeoutFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/protocol/AbstractInvoker.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/proxy/InvokerInvocationHandler.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/RpcUtils.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboCountCodec.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/filter/FutureFilter.java # dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/DubboInvokerAvilableTest.java # dubbo-rpc/dubbo-rpc-hessian/src/main/java/org/apache/dubbo/rpc/protocol/hessian/HessianProtocol.java # dubbo-rpc/dubbo-rpc-http/src/main/java/org/apache/dubbo/rpc/protocol/http/HttpProtocol.java # dubbo-rpc/dubbo-rpc-memcached/src/main/java/org/apache/dubbo/rpc/protocol/memcached/MemcachedProtocol.java # dubbo-rpc/dubbo-rpc-thrift/src/main/java/org/apache/dubbo/rpc/protocol/thrift/ThriftProtocol.java

# Conflicts: # dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorFilter.java

# Conflicts: # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ContextFilter.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java

# Conflicts: # dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java # dubbo-common/src/main/java/org/apache/dubbo/common/constants/RpcConstants.java # dubbo-compatible/src/main/java/com/alibaba/dubbo/rpc/Invocation.java # dubbo-monitor/dubbo-monitor-default/src/main/java/org/apache/dubbo/monitor/dubbo/MetricsFilter.java # dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/DefaultFuture.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/CompatibleFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericImplFilter.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/protocol/AbstractInvoker.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/proxy/InvokerInvocationHandler.java # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/support/RpcUtils.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java # dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/DubboInvokerAvilableTest.java # dubbo-rpc/dubbo-rpc-hessian/src/main/java/org/apache/dubbo/rpc/protocol/hessian/HessianProtocol.java # dubbo-rpc/dubbo-rpc-http/src/main/java/org/apache/dubbo/rpc/protocol/http/HttpProtocol.java # dubbo-rpc/dubbo-rpc-redis/src/main/java/org/apache/dubbo/rpc/protocol/redis/RedisProtocol.java # dubbo-rpc/dubbo-rpc-rmi/src/main/java/org/apache/dubbo/rpc/protocol/rmi/RmiProtocol.java # dubbo-rpc/dubbo-rpc-thrift/src/main/java/org/apache/dubbo/rpc/protocol/thrift/ThriftProtocol.java

cvictory · 2019-05-20T07:38:25Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java

-                }
-            });
-            results.put(invoker.getUrl().getServiceKey(), future);
+            results.put(invoker.getUrl().getServiceKey(), invoker.invoke(new RpcInvocation(invocation, invoker)));


In this way, it is synchronized？

Because this class is not for async?

这个是不是只是放入invoker，到后面的时候，通过get来等待？

In this way, it is synchronized？

Yes, it's synchronized, which is not the status we expect. Now, I will make it async before making the call..

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AppResponse.java

cvictory · 2019-05-21T08:01:59Z

dubbo-common/src/main/java/org/apache/dubbo/common/utils/ReflectUtils.java

+        Class<?> returnType = method.getReturnType();
+        Type genericReturnType = method.getGenericReturnType();
+        if (Future.class.isAssignableFrom(returnType)) {
+            if (genericReturnType instanceof ParameterizedType) {


https://stackoverflow.com/questions/19906614/how-to-get-generic-return-type-of-a-method-using-reflections

Do you have checked the Generic Type ?

I add a UT for this logic. Please help to check if it meets the requirement

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Result.java

dubbo-filter/dubbo-filter-cache/src/main/java/org/apache/dubbo/cache/filter/CacheFilter.java

...o-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/Response.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/FutureContext.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/RpcContext.java

cvictory · 2019-05-21T11:12:20Z

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ConsumerContextFilter.java

+    static class ConsumerContextListener implements Listener {
+        @Override
+        public void onResponse(Result appResponse, Invoker<?> invoker, Invocation invocation) {
+            RpcContext.getServerContext().setAttachments(appResponse.getAttachments());


这个和原来的RpcContext.getContext().clearAttachments() 效果不一致吧。

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ExceptionFilter.java

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/protocol/ProtocolFilterWrapper.java

codecov-io · 2019-05-22T08:58:31Z

Codecov Report

Merging #3997 into master will increase coverage by 0.06%.
The diff coverage is 67.45%.

@@             Coverage Diff              @@
##             master    #3997      +/-   ##
============================================
+ Coverage      62.7%   62.77%   +0.06%     
+ Complexity      544      475      -69     
============================================
  Files           762      766       +4     
  Lines         32758    32884     +126     
  Branches       5165     5186      +21     
============================================
+ Hits          20540    20642     +102     
+ Misses         9860     9858       -2     
- Partials       2358     2384      +26

Impacted Files	Coverage Δ	Complexity Δ
.../rpc/protocol/dubbo/LazyConnectExchangeClient.java	`58.82% <ø> (ø)`	`0 <0> (ø)`	⬇️
.../exchange/support/header/HeaderExchangeClient.java	`86.76% <ø> (ø)`	`0 <0> (ø)`	⬇️
...org/apache/dubbo/rpc/protocol/rmi/RmiProtocol.java	`60% <ø> (ø)`	`9 <0> (ø)`	⬇️
...ubbo/rpc/protocol/nativethrift/ThriftProtocol.java	`72.5% <ø> (ø)`	`8 <0> (ø)`	⬇️
...exchange/support/header/HeaderExchangeChannel.java	`81.25% <ø> (ø)`	`0 <0> (ø)`	⬇️
...a/org/apache/dubbo/remoting/RemotingException.java	`75% <ø> (ø)`	`0 <0> (ø)`	⬇️
...g/apache/dubbo/rpc/protocol/http/HttpProtocol.java	`67.85% <ø> (ø)`	`11 <0> (ø)`	⬇️
...ava/org/apache/dubbo/rpc/support/MockProtocol.java	`50% <ø> (ø)`	`0 <0> (ø)`	⬇️
.../dubbo/xml/rpc/protocol/xmlrpc/XmlRpcProtocol.java	`53.84% <ø> (ø)`	`5 <0> (ø)`	⬇️
.../dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java	`66.17% <ø> (ø)`	`0 <0> (ø)`	⬇️
... and 98 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfb6077...b155d93. Read the comment docs.

cvictory

LGTM. Some issues will be recreated in a new pr to fix.

chickenlj and others added 13 commits May 8, 2019 16:56

resolve conflicts

596b5d5

Rename some variables to avoid possible confusion between Result and …

8143d4e

…AppResponse. (apache#3889) Clear that the `Result` of the call back is actually an `AppResponse`.

Result implements future and keep Filter backward compatibility. (apa…

64081c6

…che#3916)

get CompletableFuture before thenApply

670690e

rename property to avoid possible confusion

0d0c991

add comment

db962a1

add comment

5971410

add comment

d9676b8

add comment

5c90c8f

Merge branch 'master' into consumer-thread-isolation

76862a6

# Conflicts: # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java

resolve conflicts when merging async from 3.x

93688fd

keep dubbo-demo unchanged

82ffe21

chickenlj added 7 commits May 13, 2019 16:37

delete PropertiesConfigurationTest, does not exit in master branch be…

2e9e367

…fore.

merge master to resolve conflicts

de320ba

Merge branch 'master' into consumer-thread-isolation

7381887

# Conflicts: # dubbo-monitor/dubbo-monitor-api/src/main/java/org/apache/dubbo/monitor/support/MonitorFilter.java

merge master to resolve conflicts

09f2047

remove unused imports

3194ef7

remove unused imports

2634859

chickenlj added this to the 2.7.2 milestone May 14, 2019

chickenlj added 2 commits May 15, 2019 17:46

remove unused imports

2869f6d

Merge branch 'master' into consumer-thread-isolation

5738a75

# Conflicts: # dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/ContextFilter.java # dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java

carryxyh mentioned this pull request May 16, 2019

Fix bug with CacheFilter when use async call. #3731

Closed

chickenlj added 5 commits May 16, 2019 10:37

remove unused imports

b57d818

remove unused imports

32a3d07

Fix but, replace AppResponse with AsyncRpcResult

2b8ce01

refactor doRefer in XmlRpcProtocol extension to getFrameProxy

56953eb

reuse doRefer to reduce backward influences

00aa992

cvictory self-requested a review May 17, 2019 03:09

chickenlj added 2 commits May 17, 2019 16:37

remove unused imports after merge master

fc797e2

cvictory reviewed May 20, 2019

View reviewed changes

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java Show resolved Hide resolved

cvictory reviewed May 20, 2019

View reviewed changes

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java Show resolved Hide resolved

cvictory reviewed May 20, 2019

View reviewed changes

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java Show resolved Hide resolved

cvictory reviewed May 20, 2019

View reviewed changes

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AppResponse.java Show resolved Hide resolved