Skip to content
Permalink
Browse files
[Dubbo-7367]fix too many instance bean created (#7438)
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
  • Loading branch information
zhangyz-hd committed Apr 2, 2021
1 parent 5692210 commit 23397290073381c0024706c935072dae457a5742
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 13 deletions.
@@ -16,6 +16,8 @@
*/
package org.apache.dubbo.config.spring.beans.factory.annotation;

import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.config.annotation.DubboReference;
import org.apache.dubbo.config.annotation.DubboService;
import org.apache.dubbo.config.annotation.Reference;
@@ -24,22 +26,33 @@
import org.apache.dubbo.config.spring.ServiceBean;

import com.alibaba.spring.beans.factory.annotation.AbstractAnnotationBeanPostProcessor;
import com.alibaba.spring.util.AnnotationUtils;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.annotation.InjectionMetadata;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.config.RuntimeBeanReference;
import org.springframework.beans.factory.support.AbstractBeanDefinition;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.util.ObjectUtils;

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

import static com.alibaba.spring.util.AnnotationUtils.getAttribute;
import static com.alibaba.spring.util.AnnotationUtils.getAttributes;
@@ -56,7 +69,9 @@
* @since 2.5.7
*/
public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBeanPostProcessor implements
ApplicationContextAware {
ApplicationContextAware, ApplicationListener {

private static final Logger logger = LoggerFactory.getLogger(ReferenceAnnotationBeanPostProcessor.class);

/**
* The bean name of {@link ReferenceAnnotationBeanPostProcessor}
@@ -79,6 +94,8 @@ public class ReferenceAnnotationBeanPostProcessor extends AbstractAnnotationBean

private ApplicationContext applicationContext;

private static Map<String, TreeSet<String>> referencedBeanNameIdx = new HashMap<>();

/**
* {@link com.alibaba.dubbo.config.annotation.Reference @com.alibaba.dubbo.config.annotation.Reference} has been supported since 2.7.3
* <p>
@@ -131,6 +148,8 @@ protected Object doGetInjectedBean(AnnotationAttributes attributes, Object bean,
*/
String referenceBeanName = getReferenceBeanName(attributes, injectedType);

referencedBeanNameIdx.computeIfAbsent(referencedBeanName, k -> new TreeSet<String>()).add(referenceBeanName);

ReferenceBean referenceBean = buildReferenceBeanIfAbsent(referenceBeanName, attributes, injectedType);

boolean localServiceBean = isLocalServiceBean(referencedBeanName, referenceBean, attributes);
@@ -212,9 +231,16 @@ private String generateReferenceBeanName(AnnotationAttributes attributes, Class<
if (!attributes.isEmpty()) {
beanNameBuilder.append('(');
for (Map.Entry<String, Object> entry : attributes.entrySet()) {
String value;
if ("parameters".equals(entry.getKey())) {
ArrayList<String> pairs = getParameterPairs(entry);
value = convertAttribute(pairs.stream().sorted().toArray());
} else {
value = convertAttribute(entry.getValue());
}
beanNameBuilder.append(entry.getKey())
.append('=')
.append(entry.getValue())
.append(value)
.append(',');
}
// replace the latest "," to be ")"
@@ -226,6 +252,38 @@ private String generateReferenceBeanName(AnnotationAttributes attributes, Class<
return beanNameBuilder.toString();
}

private ArrayList<String> getParameterPairs(Map.Entry<String, Object> entry) {
String[] entryValues = (String[]) entry.getValue();
ArrayList<String> pairs = new ArrayList<>();
// parameters spec is {key1,value1,key2,value2}
for (int i = 0; i < entryValues.length / 2 * 2; i = i + 2) {
pairs.add(entryValues[i] + "=" + entryValues[i + 1]);
}
return pairs;
}

private String 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 String.valueOf(attributes);
} else if (obj.getClass().isArray()) {
Object[] array = ObjectUtils.toObjectArray(obj);
String[] newArray = new String[array.length];
for (int i = 0; i < array.length; i++) {
newArray[i] = convertAttribute(array[i]);
}
return Arrays.toString(Arrays.stream(newArray).sorted().toArray());
} else {
return String.valueOf(obj);
}
}

/**
* Is Local Service bean or not?
*
@@ -344,4 +402,15 @@ public void destroy() throws Exception {
this.injectedFieldReferenceBeanCache.clear();
this.injectedMethodReferenceBeanCache.clear();
}

@Override
public void onApplicationEvent(ApplicationEvent event) {
if (event instanceof ContextRefreshedEvent) {
referencedBeanNameIdx.entrySet().stream().filter(e -> e.getValue().size() > 1).forEach(e -> {
String logPrefix = e.getKey() + " has " + e.getValue().size() + " reference instances, there are: ";
logger.warn(e.getValue().stream().collect(Collectors.joining(", ", logPrefix, "")));
});
referencedBeanNameIdx.clear();
}
}
}
@@ -16,6 +16,7 @@
*/
package org.apache.dubbo.config.spring.beans.factory.annotation;

import org.apache.dubbo.config.annotation.Argument;
import org.apache.dubbo.config.annotation.Method;
import org.apache.dubbo.config.annotation.Reference;
import org.apache.dubbo.config.spring.ReferenceBean;
@@ -123,6 +124,31 @@ public ReferenceAnnotationBeanPostProcessor referenceAnnotationBeanPostProcessor
@Reference
private HelloService helloService2;

// Instance 1
@Reference(check = false, parameters = {"a", "2", "b", "1"}, filter = {"echo"})
private HelloService helloServiceWithArray0;

// Instance 2
@Reference(check = false, parameters = {"a", "1", "b", "2"}, filter = {"echo"})
private HelloService helloServiceWithArray1;

@Reference(parameters = {"b", "2", "a", "1"}, filter = {"echo"}, check = false)
private HelloService helloServiceWithArray2;

// Instance 3
@Reference(check = false, parameters = {"a", "1"}, filter = {"echo"}, methods = {@Method(name = "sayHello", timeout = 100)})
private HelloService helloServiceWithMethod1;

@Reference(parameters = {"a", "1"}, filter = {"echo"}, check = false, methods = {@Method(name = "sayHello", timeout = 100)})
private HelloService helloServiceWithMethod2;

// Instance 4
@Reference(parameters = {"a", "1"}, filter = {"echo"}, methods = {@Method(name = "sayHello", arguments = {@Argument(callback = true, type = "String"), @Argument(callback = false, type = "int")}, timeout = 100)}, check = false)
private HelloService helloServiceWithArgument1;

@Reference(check = false, filter = {"echo"}, parameters = {"a", "1"}, methods = {@Method(name = "sayHello", timeout = 100, arguments = {@Argument(callback = false, type = "int"), @Argument(callback = true, type = "String")})})
private HelloService helloServiceWithArgument2;

@Test
public void test() throws Exception {

@@ -177,7 +203,7 @@ public void testGetReferenceBeans() {

Collection<ReferenceBean<?>> referenceBeans = beanPostProcessor.getReferenceBeans();

Assertions.assertEquals(4, referenceBeans.size());
Assertions.assertEquals(8, referenceBeans.size());

ReferenceBean<?> referenceBean = referenceBeans.iterator().next();

@@ -194,7 +220,7 @@ public void testGetInjectedFieldReferenceBeanMap() {
Map<InjectionMetadata.InjectedElement, ReferenceBean<?>> referenceBeanMap =
beanPostProcessor.getInjectedFieldReferenceBeanMap();

Assertions.assertEquals(3, referenceBeanMap.size());
Assertions.assertEquals(10, referenceBeanMap.size());

for (Map.Entry<InjectionMetadata.InjectedElement, ReferenceBean<?>> entry : referenceBeanMap.entrySet()) {

@@ -311,7 +337,7 @@ public void testReferenceBeansMethodAnnotation() {

Collection<ReferenceBean<?>> referenceBeans = beanPostProcessor.getReferenceBeans();

Assertions.assertEquals(4, referenceBeans.size());
Assertions.assertEquals(8, referenceBeans.size());

ReferenceBean<?> referenceBean = referenceBeans.iterator().next();

@@ -327,7 +327,7 @@ protected void subscribeURLs(URL url, NotifyListener listener, Set<String> servi
serviceListener.addListener(protocolServiceKey, listener);
registerServiceInstancesChangedListener(url, serviceListener);


// FIXME: This will cause redundant duplicate notifications
serviceNames.forEach(serviceName -> {
List<ServiceInstance> serviceInstances = serviceDiscovery.getInstances(serviceName);
if (CollectionUtils.isNotEmpty(serviceInstances)) {
@@ -37,6 +37,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -61,8 +62,9 @@ public class ServiceInstancesChangedListener implements ConditionalEventListener

private final Set<String> serviceNames;
private final ServiceDiscovery serviceDiscovery;
private final String registryId;
private URL url;
private Map<String, NotifyListener> listeners;
private Map<String, Set<NotifyListener>> listeners;

private Map<String, List<ServiceInstance>> allInstances;

@@ -73,6 +75,7 @@ public class ServiceInstancesChangedListener implements ConditionalEventListener
public ServiceInstancesChangedListener(Set<String> serviceNames, ServiceDiscovery serviceDiscovery) {
this.serviceNames = serviceNames;
this.serviceDiscovery = serviceDiscovery;
this.registryId = serviceDiscovery.getUrl().getParameter("id");
this.listeners = new HashMap<>();
this.allInstances = new HashMap<>();
this.serviceUrls = new HashMap<>();
@@ -188,9 +191,10 @@ private MetadataInfo getMetadataInfo(ServiceInstance instance) {
}

private void notifyAddressChanged() {
listeners.forEach((key, notifyListener) -> {
//FIXME, group wildcard match
notifyListener.notify(toUrlsWithEmpty(serviceUrls.get(key)));
listeners.forEach((key, notifyListeners) -> {
notifyListeners.forEach(notifyListener -> {
notifyListener.notify(toUrlsWithEmpty(serviceUrls.get(key)));
});
});
}

@@ -202,7 +206,7 @@ private List<URL> toUrlsWithEmpty(List<URL> urls) {
}

public void addListener(String serviceKey, NotifyListener listener) {
this.listeners.put(serviceKey, listener);
this.listeners.computeIfAbsent(serviceKey, k -> new HashSet<>()).add(listener);
}

public void removeListener(String serviceKey) {
@@ -241,6 +245,10 @@ public final boolean accept(ServiceInstancesChangedEvent event) {
return serviceNames.contains(event.getServiceName());
}

public String getRegistryId() {
return registryId;
}

@Override
public boolean equals(Object o) {
if (this == o) {
@@ -250,11 +258,11 @@ public boolean equals(Object o) {
return false;
}
ServiceInstancesChangedListener that = (ServiceInstancesChangedListener) o;
return Objects.equals(getServiceNames(), that.getServiceNames());
return Objects.equals(getServiceNames(), that.getServiceNames()) && Objects.equals(getRegistryId(), that.getRegistryId());
}

@Override
public int hashCode() {
return Objects.hash(getClass(), getServiceNames());
return Objects.hash(getClass(), getServiceNames(), getRegistryId());
}
}

0 comments on commit 2339729

Please sign in to comment.