Skip to content

[Dubbo-606] Optimize new async support for both consumer and provider side.#1876

Merged
chickenlj merged 24 commits intoapache:masterfrom
chickenlj:develop-async-response
Jun 14, 2018
Merged

[Dubbo-606] Optimize new async support for both consumer and provider side.#1876
chickenlj merged 24 commits intoapache:masterfrom
chickenlj:develop-async-response

Conversation

@chickenlj
Copy link
Copy Markdown
Contributor

@chickenlj chickenlj commented Jun 1, 2018

What is the purpose of the change

Async support for provider and consumer.

  • Consumer, support method with extra Future return type
@AsyncFor(GreetingsServices.class)
public interface GreetingsServiceAsync extends GreetingsServices {
    CompletableFuture<String> sayHi(String name);
}

We also provide a tool to do that: dubbo-async-processor

  • Provider, support async like what Servlet 3.0 async works.
public class AsyncServiceImpl implements AsyncService {

    public String sayHello(String name) {
        System.out.println("Main sayHello() method start.");
        final AsyncContext asyncContext = RpcContext.startAsync();
        new Thread(() -> {
            System.out.println("    -- Async start.");
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            asyncContext.write("Hello " + name + ", response from provider.");
            System.out.println("    -- Async end.");
        }).start();
        System.out.println("Main sayHello() method end.");
        return "hello, " + name;
    }
}

Drawbacks and following enhancements:

  • For both consumer and provider async, response can't be processed with Filter chains anymore.
  • Context still can't be restored between threads.

Thanks for @yuyijq for contributing code and ideas with #606.

Brief changelog

Verifying this change

This change has been deploy to central maven repository with a 2.6.3-SNAPSHOT version. You can see details of how to use from these two projects:
dubbo-async-processor
dubbo-samples

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

  • Make sure there is a GITHUB_issue filed 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 integration-test in test module.
  • Run mvn clean install -DskipTests & 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.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 1, 2018

Codecov Report

Merging #1876 into master will decrease coverage by 0.16%.
The diff coverage is 20.43%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1876      +/-   ##
============================================
- Coverage     49.98%   49.82%   -0.17%     
- Complexity     4561     4563       +2     
============================================
  Files           558      561       +3     
  Lines         24428    24542     +114     
  Branches       4336     4348      +12     
============================================
+ Hits          12210    12227      +17     
- Misses        10347    10436      +89     
- Partials       1871     1879       +8
Impacted Files Coverage Δ Complexity Δ
...oting/exchange/support/ExchangeHandlerAdapter.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...va/com/alibaba/dubbo/rpc/filter/ContextFilter.java 85.18% <ø> (ø) 4 <0> (ø) ⬇️
.../main/java/com/alibaba/dubbo/common/Constants.java 88.88% <ø> (ø) 1 <0> (ø) ⬇️
...ain/java/com/alibaba/dubbo/rpc/AsyncRpcResult.java 0% <0%> (ø) 0 <0> (?)
...baba/dubbo/rpc/protocol/thrift/ThriftProtocol.java 22.97% <0%> (ø) 6 <0> (ø) ⬇️
...c/main/java/com/alibaba/dubbo/rpc/AsyncResult.java 0% <0%> (ø) 0 <0> (?)
...ng/exchange/support/ExchangeHandlerDispatcher.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...baba/dubbo/rpc/proxy/InvokerInvocationHandler.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...rc/main/java/com/alibaba/dubbo/rpc/RpcContext.java 30.37% <0%> (-2.96%) 29 <0> (ø)
...n/java/com/alibaba/dubbo/rpc/AsyncContextImpl.java 0% <0%> (ø) 0 <0> (?)
... and 17 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 d2e5652...274d594. Read the comment docs.

.travis.yml Outdated
jdk:
- oraclejdk9
- oraclejdk8
- openjdk7
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we decide remove java7 support. should we have a separate pr instead of mixing in this pr?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pr should synchronize with the origin/master

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will synchronize with the origin/master branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we decide remove java7 support. should we have a separate pr instead of mixing in this pr?

If don't remove java7, travis CI would fail.

@chickenlj
Copy link
Copy Markdown
Contributor Author

I will provide enhancements with a new PR later:

  • For both consumer and provider async, response can't be processed with Filter chains anymore.
  • Context still can't be restored between threads.

@chickenlj chickenlj merged commit 0820528 into apache:master Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants