This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new cd6239112 [SCB-2808]DiscoveryTree changed to bean and not handler all
services (#3933)
cd6239112 is described below
commit cd62391121a184104b9c6a467efba8916a6cb16a
Author: liubao68 <[email protected]>
AuthorDate: Fri Sep 1 15:16:40 2023 +0800
[SCB-2808]DiscoveryTree changed to bean and not handler all services (#3933)
---
.../registry/discovery/DiscoveryTree.java | 51 ++++-----
.../registry/discovery/TestDiscoveryTree.java | 117 +++++++++++++--------
2 files changed, 92 insertions(+), 76 deletions(-)
diff --git
a/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/discovery/DiscoveryTree.java
b/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/discovery/DiscoveryTree.java
index 8d230c8b9..d040cf447 100644
---
a/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/discovery/DiscoveryTree.java
+++
b/foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/discovery/DiscoveryTree.java
@@ -19,16 +19,16 @@ package org.apache.servicecomb.registry.discovery;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import org.apache.servicecomb.foundation.common.cache.VersionedCache;
+import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.apache.servicecomb.registry.DiscoveryManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
-import com.google.common.annotations.VisibleForTesting;
-
/**
* <pre>
* DiscoveryTree is used to:
@@ -73,7 +73,7 @@ import com.google.common.annotations.VisibleForTesting;
public class DiscoveryTree {
private static final Logger LOGGER =
LoggerFactory.getLogger(DiscoveryTree.class);
- private volatile DiscoveryTreeNode root;
+ private final Map<String, Map<String, DiscoveryTreeNode>> root = new
ConcurrentHashMapEx<>();
private final Object lock = new Object();
@@ -85,21 +85,6 @@ public class DiscoveryTree {
this.discoveryManager = discoveryManager;
}
- @VisibleForTesting
- public void setRoot(DiscoveryTreeNode root) {
- this.root = root;
- }
-
- @VisibleForTesting
- public DiscoveryTreeNode getRoot() {
- return root;
- }
-
- @VisibleForTesting
- public List<DiscoveryFilter> getFilters() {
- return filters;
- }
-
@Autowired
public void setDiscoveryFilters(List<DiscoveryFilter> filters) {
this.filters = filters;
@@ -113,43 +98,45 @@ public class DiscoveryTree {
}
}
- protected boolean isMatch(VersionedCache existing, VersionedCache
inputCache) {
+ boolean isMatch(VersionedCache existing, VersionedCache inputCache) {
return existing != null && existing.isSameVersion(inputCache);
}
- protected boolean isExpired(VersionedCache existing, VersionedCache
inputCache) {
+ boolean isExpired(VersionedCache existing, VersionedCache inputCache) {
return existing == null || existing.isExpired(inputCache);
}
public DiscoveryTreeNode discovery(DiscoveryContext context, String appId,
String microserviceName) {
VersionedCache instanceVersionedCache =
this.discoveryManager.getOrCreateVersionedCache(appId, microserviceName);
- return discovery(context, instanceVersionedCache);
+ return discovery(appId, microserviceName, context, instanceVersionedCache);
}
- public DiscoveryTreeNode discovery(DiscoveryContext context, VersionedCache
inputCache) {
- DiscoveryTreeNode tmpRoot = getOrCreateRoot(inputCache);
+ DiscoveryTreeNode discovery(String appId, String microserviceName,
DiscoveryContext context,
+ VersionedCache inputCache) {
+ DiscoveryTreeNode tmpRoot = getOrCreateRoot(appId, microserviceName,
inputCache);
DiscoveryTreeNode parent = tmpRoot.children()
.computeIfAbsent(inputCache.name(), name -> new
DiscoveryTreeNode().fromCache(inputCache));
return doDiscovery(context, parent);
}
- protected DiscoveryTreeNode getOrCreateRoot(VersionedCache inputCache) {
- DiscoveryTreeNode tmpRoot = root;
+ protected DiscoveryTreeNode getOrCreateRoot(String appId, String
microserviceName, VersionedCache inputCache) {
+ DiscoveryTreeNode tmpRoot = root.computeIfAbsent(appId, k -> new
ConcurrentHashMapEx<>()).get(microserviceName);
if (isMatch(tmpRoot, inputCache)) {
return tmpRoot;
}
synchronized (lock) {
- if (isExpired(root, inputCache)) {
+ if (isExpired(tmpRoot, inputCache)) {
// not initialized or inputCache newer than root, create new root
- root = new DiscoveryTreeNode().cacheVersion(inputCache.cacheVersion());
- return root;
+ tmpRoot = new
DiscoveryTreeNode().cacheVersion(inputCache.cacheVersion());
+ root.get(appId).put(microserviceName, tmpRoot);
+ return tmpRoot;
}
- if (root.isSameVersion(inputCache)) {
+ if (tmpRoot.isSameVersion(inputCache)) {
// reuse root directly
- return root;
+ return tmpRoot;
}
}
@@ -159,7 +146,9 @@ public class DiscoveryTree {
// 3) thread 1 go on, then root is newer than inputCache
// but if create old children in new version root, it's a wrong logic
// so just create a temporary root for the inputCache, DO NOT assign to
root
- return new DiscoveryTreeNode().cacheVersion(inputCache.cacheVersion());
+ tmpRoot = new DiscoveryTreeNode().cacheVersion(inputCache.cacheVersion());
+ root.get(appId).put(microserviceName, tmpRoot);
+ return tmpRoot;
}
protected DiscoveryTreeNode doDiscovery(DiscoveryContext context,
DiscoveryTreeNode parent) {
diff --git
a/foundations/foundation-registry/src/test/java/org/apache/servicecomb/registry/discovery/TestDiscoveryTree.java
b/foundations/foundation-registry/src/test/java/org/apache/servicecomb/registry/discovery/TestDiscoveryTree.java
index d9c3106aa..c68592fbd 100644
---
a/foundations/foundation-registry/src/test/java/org/apache/servicecomb/registry/discovery/TestDiscoveryTree.java
+++
b/foundations/foundation-registry/src/test/java/org/apache/servicecomb/registry/discovery/TestDiscoveryTree.java
@@ -20,6 +20,9 @@ package org.apache.servicecomb.registry.discovery;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.servicecomb.config.ConfigUtil;
import org.apache.servicecomb.foundation.common.cache.VersionedCache;
@@ -31,6 +34,7 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
+import org.springframework.core.env.Environment;
public class TestDiscoveryTree {
DiscoveryContext context = new DiscoveryContext();
@@ -131,7 +135,7 @@ public class TestDiscoveryTree {
discoveryTree.setDiscoveryFilters(Arrays.asList(new
DiscoveryFilterForTest("g1"),
new DiscoveryFilterForTest(null), new DiscoveryFilterForTest("g2"),
new DiscoveryFilterForTest(null)));
- result = discoveryTree.discovery(context, parent);
+ result = discoveryTree.discovery("app", "service", context, parent);
Assertions.assertEquals("1.0.0-2.0.0/g1/g2", result.name());
}
@@ -150,7 +154,7 @@ public class TestDiscoveryTree {
@Test
public void discovery_filterReturnNull() {
DiscoveryManager discoveryManager = Mockito.mock(DiscoveryManager.class);
- Mockito.when(discoveryManager.getOrCreateVersionedCache(null,
null)).thenReturn(parent);
+ Mockito.when(discoveryManager.getOrCreateVersionedCache("app",
"service")).thenReturn(parent);
DiscoveryTree discoveryTree = new DiscoveryTree(discoveryManager);
DiscoveryFilter filter = new DiscoveryFilter() {
@Override
@@ -166,7 +170,7 @@ public class TestDiscoveryTree {
discoveryTree.setDiscoveryFilters(Arrays.asList(filter));
ServiceCombException exception =
Assertions.assertThrows(ServiceCombException.class,
- () -> result = discoveryTree.discovery(context, null, null));
+ () -> result = discoveryTree.discovery(context, "app", "service"));
Assertions.assertEquals(filter.getClass().getName() + " discovery return
null.", exception.getMessage());
}
@@ -199,56 +203,79 @@ public class TestDiscoveryTree {
DiscoveryTree discoveryTree = new DiscoveryTree(
new DiscoveryManager(Collections.emptyList(), List.of(new
TelnetInstancePing())));
discoveryTree.setDiscoveryFilters(Arrays.asList(f1, f2));
- result = discoveryTree.discovery(context, parent);
+ result = discoveryTree.discovery("app", "service", context, parent);
Assertions.assertEquals("second", result.data());
}
@Test
- public void avoidConcurrentProblem() {
- DiscoveryTree discoveryTree = new DiscoveryTree(
- new DiscoveryManager(Collections.emptyList(), List.of(new
TelnetInstancePing())));
- discoveryTree.setRoot(parent.cacheVersion(1));
- Assertions.assertTrue(parent.children().isEmpty());
-
- discoveryTree.discovery(context, new
VersionedCache().cacheVersion(0).name("input"));
- Assertions.assertTrue(parent.children().isEmpty());
- }
-
- @Test
- public void getOrCreateRoot_match() {
- DiscoveryTree discoveryTree = new DiscoveryTree(
- new DiscoveryManager(Collections.emptyList(), List.of(new
TelnetInstancePing())));
- discoveryTree.setRoot(parent);
-
- DiscoveryTreeNode root = discoveryTree.getOrCreateRoot(parent);
-
- Assertions.assertSame(parent, root);
- }
-
- @Test
- public void getOrCreateRoot_expired() {
- DiscoveryTree discoveryTree = new DiscoveryTree(
- new DiscoveryManager(Collections.emptyList(), List.of(new
TelnetInstancePing())));
- discoveryTree.setRoot(parent);
-
- VersionedCache inputCache = new
VersionedCache().cacheVersion(parent.cacheVersion() + 1);
- DiscoveryTreeNode root = discoveryTree.getOrCreateRoot(inputCache);
-
- Assertions.assertEquals(inputCache.cacheVersion(), root.cacheVersion());
- Assertions.assertSame(discoveryTree.getRoot(), root);
+ @SuppressWarnings("unchecked")
+ public void test_multi_service_discovery_correct() {
+ DiscoveryManager discoveryManager = Mockito.mock(DiscoveryManager.class);
+ DiscoveryTree discoveryTree = new DiscoveryTree(discoveryManager);
+ DiscoveryContext discoveryContext = new DiscoveryContext();
+
+ List<String> service1 = Arrays.asList("s11", "s12");
+ Mockito.when(discoveryManager.getOrCreateVersionedCache("app", "service1"))
+ .thenReturn(new VersionedCache().name("0+").data(service1));
+ DiscoveryTreeNode result = discoveryTree.discovery(discoveryContext,
"app", "service1");
+ Assertions.assertArrayEquals(service1.toArray(new String[0]),
+ ((List<String>) result.data()).toArray(new String[0]));
+
+ List<String> service2 = Arrays.asList("s21", "s22");
+ Mockito.when(discoveryManager.getOrCreateVersionedCache("app", "service2"))
+ .thenReturn(new VersionedCache().name("0+").data(service2));
+ result = discoveryTree.discovery(discoveryContext, "app", "service2");
+ Assertions.assertArrayEquals(service2.toArray(new String[0]),
+ ((List<String>) result.data()).toArray(new String[0]));
+
+ result = discoveryTree.discovery(discoveryContext, "app", "service1");
+ Assertions.assertArrayEquals(service1.toArray(new String[0]),
+ ((List<String>) result.data()).toArray(new String[0]));
}
@Test
- public void getOrCreateRoot_tempRoot() {
- DiscoveryTree discoveryTree = new DiscoveryTree(
- new DiscoveryManager(Collections.emptyList(), List.of(new
TelnetInstancePing())));
- discoveryTree.setRoot(parent);
-
- VersionedCache inputCache = new
VersionedCache().cacheVersion(parent.cacheVersion() - 1);
- DiscoveryTreeNode root = discoveryTree.getOrCreateRoot(inputCache);
+ @SuppressWarnings("unchecked")
+ public void test_one_service_concurrent_correct() throws Exception {
+ DiscoveryManager discoveryManager = Mockito.mock(DiscoveryManager.class);
+ DiscoveryTree discoveryTree = new DiscoveryTree(discoveryManager);
+ DiscoveryContext discoveryContext = new DiscoveryContext();
+ InstanceStatusDiscoveryFilter filter = new InstanceStatusDiscoveryFilter();
+ Environment environment = Mockito.mock(Environment.class);
+ Mockito.when(environment
+ .getProperty("servicecomb.loadbalance.filter.status.enabled",
Boolean.class, true))
+ .thenReturn(true);
+ filter.setEnvironment(environment);
+ discoveryTree.setDiscoveryFilters(List.of(filter));
+
+ StatefulDiscoveryInstance instance1 =
Mockito.mock(StatefulDiscoveryInstance.class);
+ StatefulDiscoveryInstance instance2 =
Mockito.mock(StatefulDiscoveryInstance.class);
+
+ VersionedCache expects0 = new
VersionedCache().autoCacheVersion().name("0+")
+ .data(Arrays.asList(instance1, instance2));
+ VersionedCache[] expects999 = new VersionedCache[999];
+ for (int i = 0; i < 999; i++) {
+ expects999[i] = new VersionedCache().name("0+")
+ .data(Arrays.asList(instance1, instance2)).cacheVersion(i + 1);
+ }
+ Mockito.when(discoveryManager.getOrCreateVersionedCache("app", "service1"))
+ .thenReturn(expects0, expects999);
+
+ CountDownLatch countDownLatch = new CountDownLatch(1000);
+ AtomicInteger success = new AtomicInteger(0);
+ for (int i = 0; i < 10; i++) {
+ new Thread(() -> {
+ for (int j = 0; j < 100; j++) {
+ DiscoveryTreeNode result = discoveryTree.discovery(discoveryContext,
"app", "service1");
+ if (((List<StatefulDiscoveryInstance>) result.data()).size() == 2) {
+ success.getAndIncrement();
+ }
+ countDownLatch.countDown();
+ }
+ }).start();
+ }
- Assertions.assertEquals(inputCache.cacheVersion(), root.cacheVersion());
- Assertions.assertNotSame(discoveryTree.getRoot(), root);
+ countDownLatch.await(3000, TimeUnit.MILLISECONDS);
+ Assertions.assertEquals(1000, success.get());
}
}
