Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dubbo 2.7.0的provider无法指定客户端默认async #3650

Closed
2 tasks done
haiyang1985 opened this issue Mar 14, 2019 · 10 comments
Closed
2 tasks done

Dubbo 2.7.0的provider无法指定客户端默认async #3650

haiyang1985 opened this issue Mar 14, 2019 · 10 comments
Assignees
Milestone

Comments

@haiyang1985
Copy link
Member

haiyang1985 commented Mar 14, 2019

  • I have searched the issues of this repository and believe that this is not a duplicate.
  • I have checked the FAQ of this repository and believe that this is not a duplicate.

Environment

  • Dubbo version: 2.7.0
  • Operating System version: macOS Sierra 10.12
  • Java version: 1.8.0_111

Steps to reproduce this issue

  1. start provider and specific async as true, <dubbo:service interface="" async="true">
  2. start consumer without async specific.
  3. send a request

Pls. provide [GitHub address] to reproduce this issue.

Expected Result

What do you expected from the above steps?
We are able to get the value from the CompletableFuture.
service.sayHello(input);
CompletableFuture future = RpcContext.getContext().getCompletableFuture();
future.whenComplete((retValue, error) -> {
System.out.println(retValue);
});

Actual Result

What actually happens?
It throws NPE exception as following, as we are not able to get the future.
Exception in thread "main" java.lang.NullPointerException

With investigation, it's an issue in RegistryDirectory.
Also, we are able to get async=true from providerUrl, but the querymap's async is false from consumer side. It changed the async from true to false, and caused the issue.

private URL mergeUrl(URL providerUrl) {
providerUrl = ClusterUtils.mergeUrl(providerUrl, queryMap);
}

If there is an exception, please attach the exception trace:

Just put your stack trace here!
@carryxyh
Copy link
Member

carryxyh commented Mar 14, 2019

This is a normal result.
The asynchronous side of the server should of course not affect the client.

I don't think this is a problem. Do u have any questions about it?

@haiyang1985
Copy link
Member Author

Once provider upgrade to 2.7.0, invoke be changed from async to sync, that means client cannot get result from context future.

Also, what will the async config for in provider side?

@carryxyh
Copy link
Member

Once provider upgrade to 2.7.0, invoke be changed from async to sync

I don't get your point.. Why does upgrading to 2.7 change from synchronous to asynchronous?

Also, what will the async config for in provider side?

<dubbo:service interface="..." async="true"> is enough

@haiyang1985
Copy link
Member Author

Provider 2.5.x, consumer 2.5.x, provider specific async, consumer keeps default.
<dubbo:service interface="..." async="true">
<dubbo:reference interface="...">

For consumer, they have to get value from RpcContext.
client.sayHello() returns null.
RpcContext.getContext.getFuture() returns value.

Provider 2.7.0, consumer 2.5.x, provider speicif async, consumer keeps default.
<dubbo:service interface="..." async="true">
<dubbo:reference interface="...">

For consumer, the value form RpcContext is null, this is incompatible.
client.sayHello() returns value.
RpcContext.getContext.getFuture() returns null.

@carryxyh
Copy link
Member

carryxyh commented Mar 15, 2019

Got it.

In the new version (2.7.x), the asynchronous of the provider and consumer is isolated. However, in the old version, consumer asynchronous and provider asynchronous are still bound.

Is this situation considered? @chickenlj Maybe it is a problem that we need to fix.

@haiyang1985
Copy link
Member Author

Is provider async redundancy as we are able to return CompletableFuture?

It's sync for provider.
String sayHello(String msg);

It's async for provider.
Completable sayHello(String msg);

@chickenlj chickenlj self-assigned this Mar 18, 2019
@chickenlj
Copy link
Contributor

Is provider async redundancy as we are able to return CompletableFuture?

It's sync for provider.
String sayHello(String msg);

It's async for provider.
Completable sayHello(String msg);

AsyncContext is mainly designed for String sayHello(String msg);, to make sync methods being able to achieve async on provider side.

@chickenlj chickenlj added this to the 2.7.2 milestone Mar 19, 2019
@chickenlj
Copy link
Contributor

<dubbo:service interface="..." async="true">

Provider should not be configured to allow async, user should be able to decide they want async or sync when programming without any precondition, that's what Dubbo guaranteed now.

So I think what we should do is to remove async="true" property from <dubbo:service/>.

@haiyang1985
Copy link
Member Author

For async, it's reasonable to remove it from dubbo:service/. But, are we going to remove timeout and retries also? Provider should be clear about the timeout and idempotency, then consumer. For these two keys should be kept, provider should have default values for consumer.

@fisherwangcn
Copy link

都是中国人在用,为啥总是说英文,散装英语不如直接写中文啊

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

No branches or pull requests

4 participants