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

ApplicationModel serviceName is not unique #2583

Closed
2 tasks
haiyang1985 opened this issue Sep 29, 2018 · 5 comments · Fixed by #2614
Closed
2 tasks

ApplicationModel serviceName is not unique #2583

haiyang1985 opened this issue Sep 29, 2018 · 5 comments · Fixed by #2614

Comments

@haiyang1985
Copy link
Member

haiyang1985 commented Sep 29, 2018

  • 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: all
  • Operating System version: all
  • Java version: all

Steps to reproduce this issue

  1. create two ReferenceConfig for same service with different configuration.
    <dubbo:reference id="demoService" interface="com.alibaba.dubbo.DemoService">
    </dubbo:reference>
    <dubbo:reference id="demoServiceAsync" interface="com.alibaba.dubbo.DemoService" async="true">
    </dubbo:reference>
  2. startup the consumer application.
  3. get the ConsumerModel from ApplicationModel.

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

Expected Result

ApplicationModel.allConsumerModels() should returns two ConsumerModel.

Actual Result

ApplicationModel.allConsumerModels() only returns the first ConsumerModel.

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

With future investigation, ReferenceConfig cannot put the second ConsumerModel as they are in same serviceName.

Also, this is the same issue for provider with below configuration.

<dubbo:service interface="com.alibaba.dubbo.DemoService" ref="demoService">
</dubbo:service>

<dubbo:service interface="com.alibaba.dubbo.DemoService" ref="demoService2">
</dubbo:service>

Here is my PR: #2582

@kimmking
Copy link
Member

It is dependent on how do you defind a Service:

  • a backend service is a Service
  • or a proxy service is a Service

@haiyang1985
Copy link
Member Author

Yes.
The flexibility of refer a service with different proxy is good(As above, one proxy in sync, another in async). But, it's terrible that we are not able to get the proxy we have created.
For that, we are able to cache the proxy by extend the ProxyFactory. But, the issue is that the wrapper executed without order, which result in different proxy returned.
Hopefully, the constrains be removed from the ApplicationModel cache.

@beiwei30
Copy link
Member

beiwei30 commented Oct 8, 2018

@haiyang1985 I drop a comment in #2582 (review), pls. take a look.

@beiwei30 beiwei30 mentioned this issue Oct 9, 2018
6 tasks
beiwei30 added a commit that referenced this issue Oct 9, 2018
* update README

*     #2583: ApplicationModel serviceName is not unique
@Jeff-Lv
Copy link
Contributor

Jeff-Lv commented Oct 24, 2018

I don't think this is a problem or fixing bug. Here are two reasions.

  1. Pratically, the same service is not recommended to create twice on the consumer side. If async and sync are both needed, we can do it in mehod level to config, or a util method to switch between aync and sync.

  2. most importantly, please check the two classes (URL and StringUtils), there is a method named getServiceKey, which is used commonly in Dubbo. A service can be unique under three properties whare are serviceInterface, Group, Version. So, there is no property Ref to constraint service's unique.

For example, a group rule or route rule is defined in ZK, which service is it applied to? I think, serviceKey (name, group, version) is the condition. So as the online/offline rules.

@beiwei30
Copy link
Member

@Jeff-Lv I agree with your opinion. For this particular problem, let's consider it as a limitation. User cannot configure two references with the different behavior. We will provide alternative way to user to change the behavior either via API, utilities or dynamic config.

Let's roll back the relevant change in #2646.

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 a pull request may close this issue.

4 participants