From 7d1e918b836f5f9c358b2b65c0b39ac90407ab39 Mon Sep 17 00:00:00 2001 From: caojiajun Date: Tue, 7 Nov 2023 14:06:03 +0800 Subject: [PATCH] optimize camellia-feign fallback logic --- .../core/discovery/GlobalDiscoveryEnv.java | 9 ++++ .../discovery/HashCamelliaServerSelector.java | 15 +++++- .../nim/camellia/feign/FeignCallback.java | 2 +- .../discovery/DiscoveryResourcePool.java | 53 +++++++++++++------ .../feign/naked/CamelliaNakedClient.java | 4 +- 5 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/GlobalDiscoveryEnv.java diff --git a/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/GlobalDiscoveryEnv.java b/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/GlobalDiscoveryEnv.java new file mode 100644 index 000000000..c245cb7ad --- /dev/null +++ b/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/GlobalDiscoveryEnv.java @@ -0,0 +1,9 @@ +package com.netease.nim.camellia.core.discovery; + +/** + * Created by caojiajun on 2023/11/7 + */ +public class GlobalDiscoveryEnv { + + public static boolean logInfoEnable = false; +} diff --git a/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/HashCamelliaServerSelector.java b/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/HashCamelliaServerSelector.java index b367753c8..d72f51289 100644 --- a/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/HashCamelliaServerSelector.java +++ b/camellia-core/src/main/java/com/netease/nim/camellia/core/discovery/HashCamelliaServerSelector.java @@ -1,16 +1,26 @@ package com.netease.nim.camellia.core.discovery; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.List; import java.util.concurrent.ThreadLocalRandom; public class HashCamelliaServerSelector implements CamelliaServerSelector { + private static final Logger logger = LoggerFactory.getLogger(HashCamelliaServerSelector.class); + @Override public T pick(List list, Object loadBalanceKey) { try { - if (list == null || list.isEmpty()) return null; + if (list == null || list.isEmpty()) { + return null; + } int size = list.size(); if (size == 1) { + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("pick server by hash, only one, loadBalanceKey = {}, server = {}", loadBalanceKey, list); + } return list.get(0); } int index; @@ -19,6 +29,9 @@ public T pick(List list, Object loadBalanceKey) { } else { index = Math.abs(loadBalanceKey.hashCode()) % list.size(); } + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("pick server by hash, loadBalanceKey = {}, index = {}, list = {}", loadBalanceKey, index, list); + } return list.get(index); } catch (Exception e) { return null; diff --git a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/FeignCallback.java b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/FeignCallback.java index f6c2c1986..cfcfa3b69 100644 --- a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/FeignCallback.java +++ b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/FeignCallback.java @@ -176,7 +176,7 @@ private Object invoke(Resource resource, Object loadBalanceKey, Method method, O } catch (Throwable e) { Throwable error = ExceptionUtils.onError(e); success = feignEnv.getFallbackExceptionChecker().isSkipError(error); - if (!(error instanceof CamelliaCircuitBreakerException)) { + if (!success && !(error instanceof CamelliaCircuitBreakerException)) { pool.onError(feignResource); } if (!success && failureListener != null) { diff --git a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/discovery/DiscoveryResourcePool.java b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/discovery/DiscoveryResourcePool.java index 7e2899de7..e16b77098 100644 --- a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/discovery/DiscoveryResourcePool.java +++ b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/discovery/DiscoveryResourcePool.java @@ -3,6 +3,7 @@ import com.netease.nim.camellia.core.discovery.CamelliaDiscovery; import com.netease.nim.camellia.core.discovery.CamelliaServerSelector; import com.netease.nim.camellia.core.discovery.CamelliaServerHealthChecker; +import com.netease.nim.camellia.core.discovery.GlobalDiscoveryEnv; import com.netease.nim.camellia.feign.GlobalCamelliaFeignEnv; import com.netease.nim.camellia.feign.resource.FeignDiscoveryResource; import com.netease.nim.camellia.feign.resource.FeignResource; @@ -10,9 +11,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.util.*; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -104,13 +103,19 @@ public FeignResource getResource(Object loadBalanceKey) { public void onError(FeignResource feignResource) { try { synchronized (lock) { - dynamicList.remove(feignResource); + Set set = new HashSet<>(dynamicList); + set.remove(feignResource); + List list = new ArrayList<>(set); + Collections.sort(list); + dynamicList = new ArrayList<>(list); if (dynamicList.isEmpty()) { GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(originalList)); } else { - Collections.sort(dynamicList); GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(dynamicList)); } + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("onError feignResource = {}, dynamicList = {}, originalList = {}", feignResource, dynamicList, originalList); + } } } catch (Exception e) { logger.error("onError error", e); @@ -120,10 +125,16 @@ public void onError(FeignResource feignResource) { private void add(FeignResource feignResource) { try { synchronized (lock) { - originalList.add(feignResource); - Collections.sort(originalList); - dynamicList = new ArrayList<>(originalList); - GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(dynamicList)); + Set set = new HashSet<>(originalList); + set.add(feignResource); + List list = new ArrayList<>(set); + Collections.sort(list); + originalList = new ArrayList<>(list); + dynamicList = new ArrayList<>(list); + GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(list)); + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("add feignResource = {}, dynamicList = {}, originalList = {}", feignResource, dynamicList, originalList); + } } } catch (Exception e) { logger.error("add error", e); @@ -133,16 +144,20 @@ private void add(FeignResource feignResource) { private void remove(FeignResource feignResource) { try { synchronized (lock) { - ArrayList list = new ArrayList<>(originalList); - list.remove(feignResource); - if (list.isEmpty()) { + Set set = new HashSet<>(originalList); + set.remove(feignResource); + if (set.isEmpty()) { logger.warn("last server, skip remove"); return; } + List list = new ArrayList<>(set); Collections.sort(list); - originalList = list; - dynamicList = new ArrayList<>(originalList); - GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(dynamicList)); + originalList = new ArrayList<>(list); + dynamicList = new ArrayList<>(list); + GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(list)); + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("remove feignResource = {}, dynamicList = {}, originalList = {}", feignResource, dynamicList, originalList); + } } } catch (Exception e) { logger.error("remove error", e); @@ -170,10 +185,14 @@ private void reload() { for (FeignServerInfo feignServerInfo : all) { list.add(toFeignResource(feignServerInfo)); } + Collections.sort(list); this.originalList = new ArrayList<>(list); - this.dynamicList = new ArrayList<>(originalList); + this.dynamicList = new ArrayList<>(list); - GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(dynamicList)); + GlobalCamelliaFeignEnv.register(discoveryResource, serverSelector, new ArrayList<>(list)); + if (GlobalDiscoveryEnv.logInfoEnable) { + logger.info("reload, dynamicList = {}, originalList = {}", dynamicList, originalList); + } } private FeignResource toFeignResource(FeignServerInfo feignServerInfo) { diff --git a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/naked/CamelliaNakedClient.java b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/naked/CamelliaNakedClient.java index 4dc52645b..e2d5e3d78 100644 --- a/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/naked/CamelliaNakedClient.java +++ b/camellia-feign-client/camellia-feign/src/main/java/com/netease/nim/camellia/feign/naked/CamelliaNakedClient.java @@ -440,11 +440,11 @@ private W invoke(OperationType operationType, Resource resource, R request, int } catch (CamelliaNakedClientRetriableException e) { retry ++; throwException = e; - if (feignResource != null) { + if (feignResource != null && !feignEnv.getFallbackExceptionChecker().isSkipError(e)) { resourcePool.onError(feignResource); } } catch (Exception e) { - if (feignResource != null) { + if (feignResource != null && !feignEnv.getFallbackExceptionChecker().isSkipError(e)) { resourcePool.onError(feignResource); } throw e;