From 099c0cbbd5024711ce532a2198ff264732c87d10 Mon Sep 17 00:00:00 2001 From: liuhy Date: Tue, 10 Feb 2026 08:52:29 +0800 Subject: [PATCH 1/2] fix: improve upstream cache management and add recovery test for empty events --- .../cache/UpstreamCacheManager.java | 7 +++- .../shenyu/loadbalancer/entity/Upstream.java | 2 +- .../cache/UpstreamCacheManagerTest.java | 38 +++++++++++++++++++ .../loadbalancer/entity/UpstreamTest.java | 8 ++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java index 2c13303970a6..246eace5945c 100644 --- a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java +++ b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java @@ -150,8 +150,11 @@ public void submit(final String selectorId, final List upstreamList) { // Check if the list is empty first to avoid unnecessary processing if (actualUpstreamList.isEmpty()) { - List existUpstreamList = MapUtils.computeIfAbsent(UPSTREAM_MAP, selectorId, k -> Lists.newArrayList()); - removeAllUpstreams(selectorId, existUpstreamList); + List existUpstreamList = UPSTREAM_MAP.get(selectorId); + if (Objects.nonNull(existUpstreamList)) { + removeAllUpstreams(selectorId, existUpstreamList); + } + UPSTREAM_MAP.remove(selectorId); return; } diff --git a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java index 36656e349341..d832dc28ff00 100644 --- a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java +++ b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/entity/Upstream.java @@ -509,7 +509,7 @@ public boolean equals(final Object o) { @Override public int hashCode() { - return Objects.hash(protocol, url, weight); + return Objects.hash(protocol, url); } @Override diff --git a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java index 17b449df3c5a..53290d867b85 100644 --- a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java +++ b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java @@ -278,6 +278,44 @@ public void testSubmitWithHealthCheckDisabledAndStatusFalse() { upstreamCacheManager.removeByKey(testSelectorId); } + @Test + @Order(10) + public void testSubmitCanRecoverAfterEmptyUpstreamEvent() { + final UpstreamCacheManager upstreamCacheManager = UpstreamCacheManager.getInstance(); + final String testSelectorId = "RECOVER_AFTER_EMPTY_EVENT_TEST"; + + List initialList = new ArrayList<>(1); + initialList.add(Upstream.builder() + .protocol("http://") + .url("recover-upstream:8080") + .status(true) + .healthCheckEnabled(false) + .build()); + upstreamCacheManager.submit(testSelectorId, initialList); + List firstSubmitResult = upstreamCacheManager.findUpstreamListBySelectorId(testSelectorId); + Assertions.assertNotNull(firstSubmitResult); + Assertions.assertFalse(firstSubmitResult.isEmpty()); + + upstreamCacheManager.submit(testSelectorId, new ArrayList<>()); + List afterEmptySubmitResult = upstreamCacheManager.findUpstreamListBySelectorId(testSelectorId); + Assertions.assertTrue(Objects.isNull(afterEmptySubmitResult) || afterEmptySubmitResult.isEmpty()); + + List recoveredList = new ArrayList<>(1); + recoveredList.add(Upstream.builder() + .protocol("http://") + .url("recover-upstream:8080") + .status(true) + .healthCheckEnabled(false) + .build()); + upstreamCacheManager.submit(testSelectorId, recoveredList); + List secondSubmitResult = upstreamCacheManager.findUpstreamListBySelectorId(testSelectorId); + Assertions.assertNotNull(secondSubmitResult); + Assertions.assertFalse(secondSubmitResult.isEmpty()); + Assertions.assertTrue(secondSubmitResult.stream().anyMatch(upstream -> "recover-upstream:8080".equals(upstream.getUrl()))); + + upstreamCacheManager.removeByKey(testSelectorId); + } + /** * Helper method to get the UpstreamCheckTask using reflection. */ diff --git a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/entity/UpstreamTest.java b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/entity/UpstreamTest.java index 9a9146f6b0b2..ab7d009707b5 100644 --- a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/entity/UpstreamTest.java +++ b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/entity/UpstreamTest.java @@ -73,7 +73,15 @@ public void upstreamTest() { .weight(1) .status(true) .build(); + Upstream upstream4 = Upstream.builder() + .protocol("https://") + .url("url") + .weight(2) + .status(true) + .build(); Assertions.assertEquals(upstream2, upstream3); + Assertions.assertEquals(upstream2, upstream4); + Assertions.assertEquals(upstream2.hashCode(), upstream4.hashCode()); Assertions.assertNotNull(upstream2.toString()); Assertions.assertTrue(upstream2.hashCode() >= 0); } From a144a863beea8b61df03e50ee584005720faf0dd Mon Sep 17 00:00:00 2001 From: liuhy Date: Tue, 10 Feb 2026 09:28:23 +0800 Subject: [PATCH 2/2] fix: streamline upstream removal process and add test for empty event handling --- .../cache/UpstreamCacheManager.java | 12 +---- .../cache/UpstreamCacheManagerTest.java | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java index 246eace5945c..b39124228519 100644 --- a/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java +++ b/shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java @@ -26,7 +26,6 @@ import org.apache.shenyu.common.utils.Singleton; import org.apache.shenyu.loadbalancer.entity.Upstream; -import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -150,11 +149,7 @@ public void submit(final String selectorId, final List upstreamList) { // Check if the list is empty first to avoid unnecessary processing if (actualUpstreamList.isEmpty()) { - List existUpstreamList = UPSTREAM_MAP.get(selectorId); - if (Objects.nonNull(existUpstreamList)) { - removeAllUpstreams(selectorId, existUpstreamList); - } - UPSTREAM_MAP.remove(selectorId); + removeByKey(selectorId); return; } @@ -182,11 +177,6 @@ private void initializeUpstreamHealthStatus(final List upstreamList) { }); } - private void removeAllUpstreams(final String selectorId, final List existUpstreamList) { - List toRemove = new ArrayList<>(existUpstreamList); - toRemove.forEach(up -> task.triggerRemoveOne(selectorId, up)); - } - private void processOfflineUpstreams(final String selectorId, final List offlineUpstreamList, final List existUpstreamList) { Map currentUnhealthyMap = getCurrentUnhealthyMap(selectorId); diff --git a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java index 53290d867b85..94bfc86fd4c9 100644 --- a/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java +++ b/shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java @@ -316,6 +316,50 @@ public void testSubmitCanRecoverAfterEmptyUpstreamEvent() { upstreamCacheManager.removeByKey(testSelectorId); } + @Test + @Order(11) + public void testSubmitEmptyEventClearsUnhealthyState() { + final UpstreamCacheManager upstreamCacheManager = UpstreamCacheManager.getInstance(); + final String testSelectorId = "EMPTY_EVENT_CLEARS_UNHEALTHY_TEST"; + + List offlineList = new ArrayList<>(1); + offlineList.add(Upstream.builder() + .protocol("http://") + .url("stale-upstream:8080") + .status(false) + .healthCheckEnabled(true) + .build()); + upstreamCacheManager.submit(testSelectorId, offlineList); + + UpstreamCheckTask task = getUpstreamCheckTask(upstreamCacheManager); + if (Objects.nonNull(task)) { + List unhealthyBeforeEmpty = task.getUnhealthyUpstream().get(testSelectorId); + Assertions.assertNotNull(unhealthyBeforeEmpty); + Assertions.assertFalse(unhealthyBeforeEmpty.isEmpty()); + } + + upstreamCacheManager.submit(testSelectorId, new ArrayList<>()); + + if (Objects.nonNull(task)) { + List unhealthyAfterEmpty = task.getUnhealthyUpstream().get(testSelectorId); + Assertions.assertTrue(Objects.isNull(unhealthyAfterEmpty) || unhealthyAfterEmpty.isEmpty()); + } + + List recoveredList = new ArrayList<>(1); + recoveredList.add(Upstream.builder() + .protocol("http://") + .url("stale-upstream:8080") + .status(true) + .healthCheckEnabled(false) + .build()); + upstreamCacheManager.submit(testSelectorId, recoveredList); + List finalResult = upstreamCacheManager.findUpstreamListBySelectorId(testSelectorId); + Assertions.assertNotNull(finalResult); + Assertions.assertFalse(finalResult.isEmpty()); + + upstreamCacheManager.removeByKey(testSelectorId); + } + /** * Helper method to get the UpstreamCheckTask using reflection. */