fix router concurrent bug&fix spring-retry circuit breaker not work bug (#679)

CHANGELOG.md

@@ -1,2 +1,4 @@
 # Change Log
 ---
+
+- [fix router concurrent bug&fix spring-retry circuit breaker not work bug](https://github.com/Tencent/spring-cloud-tencent/pull/679)

pom.xml

@@ -89,7 +89,7 @@
 
 	<properties>
 		<!-- Project revision -->
-		<revision>1.8.1-Hoxton.SR12</revision>
+		<revision>1.8.2-Hoxton.SR12-SNAPSHOT</revision>
 
 		<!-- Spring Cloud -->
 		<spring.cloud.version>Hoxton.SR12</spring.cloud.version>

spring-cloud-starter-tencent-polaris-router/src/main/java/com/tencent/cloud/polaris/router/PolarisLoadBalancerCompositeRule.java

@@ -20,6 +20,7 @@ package com.tencent.cloud.polaris.router;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import com.netflix.client.config.IClientConfig;
 import com.netflix.loadbalancer.AbstractLoadBalancerRule;
@@ -37,6 +38,7 @@ import com.tencent.cloud.common.metadata.MetadataContext;
 import com.tencent.cloud.common.pojo.PolarisServer;
 import com.tencent.cloud.common.util.JacksonUtils;
 import com.tencent.cloud.polaris.loadbalancer.LoadBalancerUtils;
+import com.tencent.cloud.polaris.loadbalancer.PolarisLoadBalancer;
 import com.tencent.cloud.polaris.loadbalancer.PolarisWeightedRule;
 import com.tencent.cloud.polaris.loadbalancer.config.PolarisLoadBalancerProperties;
 import com.tencent.cloud.polaris.router.spi.RouterRequestInterceptor;
@@ -84,6 +86,8 @@ public class PolarisLoadBalancerCompositeRule extends AbstractLoadBalancerRule {
 
 	private final AbstractLoadBalancerRule delegateRule;
 
+	private final AtomicBoolean initializedLoadBalancerToDelegateRule = new AtomicBoolean(false);
+
 	public PolarisLoadBalancerCompositeRule(RouterAPI routerAPI,
 			PolarisLoadBalancerProperties polarisLoadBalancerProperties,
 			IClientConfig iClientConfig,
@@ -111,22 +115,31 @@ public class PolarisLoadBalancerCompositeRule extends AbstractLoadBalancerRule {
 
 	@Override
 	public Server choose(Object key) {
-		// 1. get all servers
+		ILoadBalancer loadBalancer = getLoadBalancer();
-		List<Server> allServers = getLoadBalancer().getReachableServers();
+
+		if (!(loadBalancer instanceof PolarisLoadBalancer)) {
+			return loadBalancer.chooseServer(key);
+		}
+
+		// set load balancer to delegate rule
+		if (!initializedLoadBalancerToDelegateRule.compareAndSet(false, true)) {
+			delegateRule.setLoadBalancer(loadBalancer);
+		}
+
+		PolarisLoadBalancer polarisLoadBalancer = (PolarisLoadBalancer) loadBalancer;
+
+		// 1. get all servers from polaris client
+		List<Server> allServers = polarisLoadBalancer.getReachableServersWithoutCache();
 		if (CollectionUtils.isEmpty(allServers)) {
 			return null;
 		}
 
-		ILoadBalancer loadBalancer = new SimpleLoadBalancer();
 		// 2. filter by router
+		List<Server> serversAfterRouter;
 		if (key instanceof PolarisRouterContext) {
 			// router implement for Feign and scg
 			PolarisRouterContext routerContext = (PolarisRouterContext) key;
-			List<Server> serversAfterRouter = doRouter(allServers, routerContext);
+			serversAfterRouter = doRouter(allServers, routerContext);
-			// 3. filter by load balance.
-			// A LoadBalancer needs to be regenerated for each request,
-			// because the list of servers may be different after filtered by router
-			loadBalancer.addServers(serversAfterRouter);
 		}
 		else if (key instanceof HttpRequest) {
 			// router implement for rest template
@@ -135,20 +148,20 @@ public class PolarisLoadBalancerCompositeRule extends AbstractLoadBalancerRule {
 			String routerContextStr = request.getHeaders().getFirst(RouterConstant.HEADER_ROUTER_CONTEXT);
 
 			if (StringUtils.isEmpty(routerContextStr)) {
-				loadBalancer.addServers(allServers);
+				serversAfterRouter = allServers;
 			}
 			else {
 				PolarisRouterContext routerContext = JacksonUtils.deserialize(UriEncoder.decode(routerContextStr),
 						PolarisRouterContext.class);
-				List<Server> serversAfterRouter = doRouter(allServers, routerContext);
+				serversAfterRouter = doRouter(allServers, routerContext);
-				loadBalancer.addServers(serversAfterRouter);
 			}
 		}
 		else {
-			loadBalancer.addServers(allServers);
+			serversAfterRouter = allServers;
 		}
 
-		delegateRule.setLoadBalancer(loadBalancer);
+		// 3. put filtered servers to thread local, so delegate rule choose servers from filtered servers.
+		polarisLoadBalancer.addServers(serversAfterRouter);
 
 		return delegateRule.choose(key);
 	}

spring-cloud-starter-tencent-polaris-router/src/main/java/com/tencent/cloud/polaris/router/SimpleLoadBalancer.java

@@ -1,73 +0,0 @@
-/*
- * Tencent is pleased to support the open source community by making Spring Cloud Tencent available.
- *
- * Copyright (C) 2019 THL A29 Limited, a Tencent company. All rights reserved.
- *
- * Licensed under the BSD 3-Clause License (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * https://opensource.org/licenses/BSD-3-Clause
- *
- * Unless required by applicable law or agreed to in writing, software distributed
- * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
- * CONDITIONS OF ANY KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations under the License.
- *
- */
-
-package com.tencent.cloud.polaris.router;
-
-import java.util.Collections;
-import java.util.List;
-
-import com.netflix.loadbalancer.ILoadBalancer;
-import com.netflix.loadbalancer.Server;
-
-/**
- * Simple load balancer only for getting and setting servers.
- *
- * @author lepdou 2022-05-17
- */
-public class SimpleLoadBalancer implements ILoadBalancer {
-	private List<Server> servers;
-
-	@Override
-	public void addServers(List<Server> newServers) {
-		this.servers = newServers;
-	}
-
-	@Override
-	public Server chooseServer(Object key) {
-		return null;
-	}
-
-	@Override
-	public void markServerDown(Server server) {
-
-	}
-
-	@Override
-	public List<Server> getServerList(boolean availableOnly) {
-		if (servers == null) {
-			return Collections.emptyList();
-		}
-		return Collections.unmodifiableList(servers);
-	}
-
-	@Override
-	public List<Server> getReachableServers() {
-		if (servers == null) {
-			return Collections.emptyList();
-		}
-		return Collections.unmodifiableList(servers);
-	}
-
-	@Override
-	public List<Server> getAllServers() {
-		if (servers == null) {
-			return Collections.emptyList();
-		}
-		return Collections.unmodifiableList(servers);
-	}
-}

spring-cloud-starter-tencent-polaris-router/src/test/java/com/tencent/cloud/polaris/router/SimpleLoadBalancerTest.java

@@ -1,72 +0,0 @@
-/*
- * Tencent is pleased to support the open source community by making Spring Cloud Tencent available.
- *
- * Copyright (C) 2019 THL A29 Limited, a Tencent company. All rights reserved.
- *
- * Licensed under the BSD 3-Clause License (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * https://opensource.org/licenses/BSD-3-Clause
- *
- * Unless required by applicable law or agreed to in writing, software distributed
- * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
- * CONDITIONS OF ANY KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations under the License.
- *
- */
-
-package com.tencent.cloud.polaris.router;
-
-import java.util.LinkedList;
-import java.util.List;
-
-import com.netflix.loadbalancer.Server;
-import org.junit.Assert;
-import org.junit.Test;
-import org.mockito.Mockito;
-
-/**
- * test for {@link SimpleLoadBalancer}.
- *
- * @author lepdou 2022-05-26
- */
-public class SimpleLoadBalancerTest {
-
-	@Test
-	public void testSetterGetter() {
-		List<Server> servers = new LinkedList<>();
-		servers.add(Mockito.mock(Server.class));
-		servers.add(Mockito.mock(Server.class));
-		servers.add(Mockito.mock(Server.class));
-		servers.add(Mockito.mock(Server.class));
-		servers.add(Mockito.mock(Server.class));
-
-		SimpleLoadBalancer simpleLoadBalancer = new SimpleLoadBalancer();
-
-		simpleLoadBalancer.addServers(servers);
-
-		List<Server> allServers = simpleLoadBalancer.getAllServers();
-		List<Server> reachableServers = simpleLoadBalancer.getReachableServers();
-		List<Server> availableServers = simpleLoadBalancer.getServerList(true);
-
-		Assert.assertEquals(servers.size(), allServers.size());
-		Assert.assertEquals(servers.size(), reachableServers.size());
-		Assert.assertEquals(servers.size(), availableServers.size());
-	}
-
-	@Test
-	public void testSetNull() {
-		SimpleLoadBalancer simpleLoadBalancer = new SimpleLoadBalancer();
-
-		simpleLoadBalancer.addServers(null);
-
-		List<Server> allServers = simpleLoadBalancer.getAllServers();
-		List<Server> reachableServers = simpleLoadBalancer.getReachableServers();
-		List<Server> availableServers = simpleLoadBalancer.getServerList(true);
-
-		Assert.assertEquals(0, allServers.size());
-		Assert.assertEquals(0, reachableServers.size());
-		Assert.assertEquals(0, availableServers.size());
-	}
-}

spring-cloud-tencent-dependencies/pom.xml

@@ -70,7 +70,7 @@
 	</developers>
 
 	<properties>
-		<revision>1.8.1-Hoxton.SR12</revision>
+		<revision>1.8.2-Hoxton.SR12-SNAPSHOT</revision>
 		<polaris.version>1.9.1</polaris.version>
 		<logback.version>1.2.11</logback.version>
 		<mocktio.version>4.5.1</mocktio.version>

spring-cloud-tencent-polaris-loadbalancer/src/main/java/com/tencent/cloud/polaris/loadbalancer/PolarisLoadBalancer.java

@@ -52,6 +52,7 @@ import org.springframework.util.CollectionUtils;
  * @author Haotian Zhang
  */
 public class PolarisLoadBalancer extends DynamicServerListLoadBalancer<Server> {
+	private static final ThreadLocal<List<Server>> THREAD_CACHE_SERVERS = new ThreadLocal<>();
 
 	private final ConsumerAPI consumerAPI;
 
@@ -64,8 +65,21 @@ public class PolarisLoadBalancer extends DynamicServerListLoadBalancer<Server> {
 		this.polarisLoadBalancerProperties = properties;
 	}
 
+	@Override
+	public void addServers(List<Server> servers) {
+		THREAD_CACHE_SERVERS.set(servers);
+	}
+
 	@Override
 	public List<Server> getReachableServers() {
+		// Get servers first from the thread context
+		if (!CollectionUtils.isEmpty(THREAD_CACHE_SERVERS.get())) {
+			return THREAD_CACHE_SERVERS.get();
+		}
+		return getReachableServersWithoutCache();
+	}
+
+	public List<Server> getReachableServersWithoutCache() {
 		ServiceInstances serviceInstances;
 		if (polarisLoadBalancerProperties.getDiscoveryType().equals(ContextConstant.POLARIS)) {
 			serviceInstances = getPolarisDiscoveryServiceInstances();