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-7367]fix too many instance bean created #7438

Merged
merged 21 commits into from
Apr 2, 2021

Conversation

zhangyz-hd
Copy link
Contributor

@zhangyz-hd zhangyz-hd commented Mar 24, 2021

What is the purpose of the change

初步修复 #7367 的问题
当多处用注解订阅同一个服务,并使用parameters时,如

@DubboReference(version = "2.0", check = false, parameters = {"a", "1"})

实际会产生多个Reference示例

Brief changelog

1,修复generateReferenceBeanName过程中未处理Array类型的entry问题
2,如果产生多个相同“INTERFACE:VERSION:GROUP”的Reference实例,则在启动日志中增加WARN信息,帮助用户分析
日志示例

[25/03/21 07:18:23:790 CST] main  WARN annotation.ReferenceAnnotationBeanPostProcessor:  [DUBBO] ServiceBean:org.apache.dubbo.demo.DemoServiceA1:2.0:Group1 has 2 reference instances, there are: @Reference(check=false,group=Group1,parameters=[a=1,b=2],version=2.0) org.apache.dubbo.demo.DemoServiceA1, @Reference(check=false,group=Group1,parameters=[a=2,b=1],version=2.0) org.apache.dubbo.demo.DemoServiceA1, dubbo version: , current host: 192.168.1.178

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.

1,修复相同parameters当产生多个Reference的问题。
2,如果相同服务以不同参数订阅多次,则启动时增加WARN日志
@zhangyz-hd zhangyz-hd changed the title [Dubbo-7367]fix too many instance bean created WIP[Dubbo-7367]fix too many instance bean created Mar 25, 2021
if ("parameters".equals(entry.getKey())) {
// parameters spec is {key1,value1,key2,value2}
ArrayList<String> kvList = new ArrayList<>();
for (int i = 0; i < entryValues.length / 2 * 2; i = i + 2) {
Copy link
Member

Choose a reason for hiding this comment

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

i < entryValues.length / 2 * 2 changed to i < entryValues.length.

Copy link
Contributor Author

@zhangyz-hd zhangyz-hd Mar 25, 2021

Choose a reason for hiding this comment

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

This is to avoid mistake useage like {key1,value1,key2,value2,key3}

@Override
public void onApplicationEvent(ApplicationEvent event) {
if (event instanceof ContextRefreshedEvent) {
referencedBeanNameIdx.entrySet().stream().filter(e -> e.getValue().size() > 1).forEach(e -> {
Copy link
Member

Choose a reason for hiding this comment

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

release referencedBeanNameIdx resource after app started.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

release referencedBeanNameIdx after used.
@codecov-io
Copy link

codecov-io commented Mar 25, 2021

Codecov Report

Merging #7438 (4eacf5b) into master (2584cab) will decrease coverage by 0.34%.
The diff coverage is 75.55%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7438      +/-   ##
============================================
- Coverage     59.39%   59.04%   -0.35%     
- Complexity      466      502      +36     
============================================
  Files          1043     1079      +36     
  Lines         42516    43368     +852     
  Branches       6226     6324      +98     
============================================
+ Hits          25251    25606     +355     
- Misses        14472    14920     +448     
- Partials       2793     2842      +49     
Impacted Files Coverage Δ Complexity Δ
...ubbo/registry/client/ServiceDiscoveryRegistry.java 51.78% <ø> (ø) 0.00 <0.00> (ø)
...vent/listener/ServiceInstancesChangedListener.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...notation/ReferenceAnnotationBeanPostProcessor.java 91.80% <94.44%> (+0.99%) 0.00 <0.00> (ø)
...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java 26.86% <0.00%> (-52.24%) 0.00% <0.00%> (ø%)
...va/org/apache/dubbo/rpc/support/AccessLogData.java 53.16% <0.00%> (-37.98%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/all/AllChannelHandler.java 68.96% <0.00%> (-27.59%) 0.00% <0.00%> (ø%)
.../cluster/configurator/parser/model/ConfigItem.java 48.00% <0.00%> (-24.00%) 0.00% <0.00%> (ø%)
...dubbo/common/status/support/LoadStatusChecker.java 46.15% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...rg/apache/dubbo/remoting/utils/PayloadDropper.java 76.92% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/WrappedChannelHandler.java 43.47% <0.00%> (-15.22%) 0.00% <0.00%> (ø%)
... and 85 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 2584cab...4eacf5b. Read the comment docs.

@zonghaishang zonghaishang requested a review from chickenlj March 25, 2021 06:30
只处理String类型的array,对Method[]暂时不处理
支持处理methods属性和arguments属性
调整ServiceInstancesChangedListener的事件通知。所有的directory都能通知到
@zhangyz-hd zhangyz-hd changed the title WIP[Dubbo-7367]fix too many instance bean created WIP [Dubbo-7367]fix too many instance bean created Mar 26, 2021
private URL url;
private Map<String, NotifyListener> listeners;
private Map<String, List<NotifyListener>> listeners;
Copy link
Member

Choose a reason for hiding this comment

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

List改成Set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK~

@zhangyz-hd zhangyz-hd requested a review from zonghaishang March 26, 2021 11:49
@zhangyz-hd zhangyz-hd changed the title WIP [Dubbo-7367]fix too many instance bean created [Dubbo-7367]fix too many instance bean created Mar 26, 2021
@zonghaishang zonghaishang self-assigned this Mar 27, 2021
//handle method array, generic array
if (value != null && value.getClass().isArray()) {
Object[] array = ObjectUtils.toObjectArray(value);
value = Arrays.toString(array);
Copy link
Member

Choose a reason for hiding this comment

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

It seems that @Method's @Argument array is not handled properly.

@DubboReference(methods = @Method(name = "sayHello", timeout = 100, arguments = @Argument(index = 0, callback = true)))

Some code snippets:

for (String key : sortedAttrKeys) {
      Object value = attributes.get(key);
      value = convertAttribute(value);
      beanNameBuilder.append(key)
              .append('=')
              .append(value)
              .append(',');
}
    private Object convertAttribute(Object obj) {
        if (obj == null) {
            return null;
        }
        if (obj instanceof Annotation) {
            AnnotationAttributes attributes = AnnotationUtils.getAnnotationAttributes((Annotation) obj, true);
            for (Map.Entry<String, Object> entry : attributes.entrySet()) {
                entry.setValue(convertAttribute(entry.getValue()));
            }
            return attributes;
        } else if (obj.getClass().isArray()) {
            Object[] array = ObjectUtils.toObjectArray(obj);
            Object[] newArray = new Object[array.length];
            for (int i = 0; i < array.length; i++) {
                newArray[i] = convertAttribute(array[i]);
            }
            return Arrays.toString(newArray);
        } else {
            return obj;
        }
    }

@kylixs
Copy link
Member

kylixs commented Apr 2, 2021

LGTM

@zonghaishang zonghaishang merged commit 2339729 into apache:master Apr 2, 2021
@zhangyz-hd zhangyz-hd deleted the 210324-fix-7367-step-1 branch July 5, 2021 02:22
vio-lin pushed a commit to vio-lin/incubator-dubbo that referenced this pull request Feb 22, 2023
1,修复相同parameters当产生多个Reference的问题。
2,如果相同服务以不同参数订阅多次,则启动时增加WARN日志

* Update ReferenceAnnotationBeanPostProcessor.java

修改日志格式

* Update ReferenceAnnotationBeanPostProcessor.java

release referencedBeanNameIdx after used.

* Update ReferenceAnnotationBeanPostProcessorTest.java

add UT

* Update ReferenceAnnotationBeanPostProcessor.java

只处理String类型的array,对Method[]暂时不处理

* 优化generateReferenceBeanName

支持处理methods属性和arguments属性

* methods和arguments需要排序

* Update ReferenceAnnotationUtils.java

use lambda

* update ServiceInstancesChangedListener

调整ServiceInstancesChangedListener的事件通知。所有的directory都能通知到

* Update ServiceInstancesChangedListener.java

listeners内部改为HashSet

* update generateReferenceBeanName

generateReferenceBeanName改用3.0-preview逻辑

* remote println

* Update ReferenceAnnotationBeanPostProcessor.java

remote unused private method

* Update ReferenceAnnotationBeanPostProcessor.java

* Update ReferenceAnnotationBeanPostProcessorTest.java

update UT

* update UT

* revert to use ReferenceAnnotationUtils

* 使用来自kylixs的convertAttribute方法

* organize imports & update UT

* update UT

* update ReferenceAnnotationBeanPostProcessor & UT

(cherry picked from commit 2339729)
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