This is an automated email from the ASF dual-hosted git repository.
yiji pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/master by this push:
new 2339729 [Dubbo-7367]fix too many instance bean created (#7438)
2339729 is described below
commit 23397290073381c0024706c935072dae457a5742
Author: 张远征hd <[email protected]>
AuthorDate: Fri Apr 2 16:07:18 2021 +0800
[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
---
.../ReferenceAnnotationBeanPostProcessor.java | 73 +++++++++++++++++++++-
.../ReferenceAnnotationBeanPostProcessorTest.java | 32 +++++++++-
.../registry/client/ServiceDiscoveryRegistry.java | 2 +-
.../listener/ServiceInstancesChangedListener.java | 22 ++++---
4 files changed, 116 insertions(+), 13 deletions(-)
diff --git
a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java
b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java
index 07de2ee..9a9dfac 100644
---
a/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java
+++
b/dubbo-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessor.java
@@ -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,6 +26,7 @@ import org.apache.dubbo.config.spring.ReferenceBean;
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;
@@ -31,15 +34,25 @@ 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 @@ import static org.springframework.util.StringUtils.hasText;
* @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 @@ public class ReferenceAnnotationBeanPostProcessor extends
AbstractAnnotationBean
*/
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 @@ public class ReferenceAnnotationBeanPostProcessor extends
AbstractAnnotationBean
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 @@ public class ReferenceAnnotationBeanPostProcessor extends
AbstractAnnotationBean
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 class ReferenceAnnotationBeanPostProcessor extends
AbstractAnnotationBean
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();
+ }
+ }
}
diff --git
a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
index 6007e81..d262850 100644
---
a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
+++
b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/beans/factory/annotation/ReferenceAnnotationBeanPostProcessorTest.java
@@ -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 class ReferenceAnnotationBeanPostProcessorTest {
@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 class ReferenceAnnotationBeanPostProcessorTest {
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 class ReferenceAnnotationBeanPostProcessorTest {
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 class ReferenceAnnotationBeanPostProcessorTest {
Collection<ReferenceBean<?>> referenceBeans =
beanPostProcessor.getReferenceBeans();
- Assertions.assertEquals(4, referenceBeans.size());
+ Assertions.assertEquals(8, referenceBeans.size());
ReferenceBean<?> referenceBean = referenceBeans.iterator().next();
diff --git
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java
index 7c59c21..ac0b526 100644
---
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java
+++
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java
@@ -327,7 +327,7 @@ public class ServiceDiscoveryRegistry implements Registry {
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)) {
diff --git
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java
index 55af495..d40fd01 100644
---
a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java
+++
b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java
@@ -37,6 +37,7 @@ import
org.apache.dubbo.registry.client.metadata.store.RemoteMetadataServiceImpl
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 @@ public class ServiceInstancesChangedListener implements
ConditionalEventListener
}
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 @@ public class ServiceInstancesChangedListener implements
ConditionalEventListener
}
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 class ServiceInstancesChangedListener implements
ConditionalEventListener
return serviceNames.contains(event.getServiceName());
}
+ public String getRegistryId() {
+ return registryId;
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -250,11 +258,11 @@ public class ServiceInstancesChangedListener implements
ConditionalEventListener
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());
}
}
