This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 49720801a9 fix(admin): prevent scaling rules with the same metric from
overwriting each other (#7875)
49720801a9 is described below
commit 49720801a9de605ad97b62189ecd6894be967fc2
Author: Joker <[email protected]>
AuthorDate: Wed Oct 21 10:53:42 2026 +0800
fix(admin): prevent scaling rules with the same metric from overwriting
each other (#7875)
Co-authored-by: Mrmr <[email protected]>
---
.../monitor/subject/cache/ScaleRuleCache.java | 29 ++--
.../admin/service/impl/ScaleRuleServiceImpl.java | 9 +-
.../monitor/subject/cache/ScaleRuleCacheTest.java | 146 +++++++++++++++++++++
.../shenyu/admin/service/ScaleRuleServiceTest.java | 58 ++++++--
4 files changed, 213 insertions(+), 29 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCache.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCache.java
index d206c4d711..2ec1df4ac2 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCache.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCache.java
@@ -20,9 +20,9 @@ package org.apache.shenyu.admin.scale.monitor.subject.cache;
import org.apache.shenyu.admin.model.entity.ScaleRuleDO;
import org.springframework.stereotype.Component;
-import java.util.HashSet;
+import java.util.Comparator;
import java.util.List;
-import java.util.Set;
+import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
@Component
@@ -37,7 +37,7 @@ public class ScaleRuleCache {
*/
public void initialize(final List<ScaleRuleDO> rules) {
ruleCache.clear();
- rules.forEach(rule -> ruleCache.put(rule.getMetricName(), rule));
+ rules.forEach(rule -> ruleCache.put(rule.getId(), rule));
}
/**
@@ -46,7 +46,7 @@ public class ScaleRuleCache {
* @param rule rule
*/
public void addOrUpdateRuleToCache(final ScaleRuleDO rule) {
- ruleCache.put(rule.getMetricName(), rule);
+ ruleCache.put(rule.getId(), rule);
}
/**
@@ -55,7 +55,11 @@ public class ScaleRuleCache {
* @param metricNames metricNames
*/
public void removeRulesFromCache(final List<String> metricNames) {
- metricNames.forEach(ruleCache::remove);
+ ruleCache.forEach((id, rule) -> {
+ if (metricNames.contains(rule.getMetricName())) {
+ ruleCache.remove(id, rule);
+ }
+ });
}
/**
@@ -64,12 +68,7 @@ public class ScaleRuleCache {
* @param ids rule primary keys
*/
public void removeRulesByIdsFromCache(final List<String> ids) {
- final Set<String> idSet = new HashSet<>(ids);
- ruleCache.forEach((metricName, rule) -> {
- if (idSet.contains(rule.getId())) {
- ruleCache.remove(metricName, rule);
- }
- });
+ ids.forEach(ruleCache::remove);
}
/**
@@ -82,12 +81,16 @@ public class ScaleRuleCache {
}
/**
- * getRuleByMetricName.
+ * Gets the highest-priority rule for a metric, breaking equal priorities
by rule ID.
*
* @param metricName metricName
* @return ScaleRuleDO
*/
public ScaleRuleDO getRuleByMetricName(final String metricName) {
- return ruleCache.get(metricName);
+ return ruleCache.values().stream()
+ .filter(rule -> Objects.equals(metricName,
rule.getMetricName()))
+ .min(Comparator.comparing(ScaleRuleDO::getSort,
Comparator.nullsLast(Integer::compareTo))
+ .thenComparing(ScaleRuleDO::getId))
+ .orElse(null);
}
}
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ScaleRuleServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ScaleRuleServiceImpl.java
index faa97abc36..56994834b8 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ScaleRuleServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ScaleRuleServiceImpl.java
@@ -34,7 +34,6 @@ import
org.springframework.transaction.support.TransactionSynchronization;
import
org.springframework.transaction.support.TransactionSynchronizationManager;
import java.util.List;
-import java.util.Objects;
import java.util.stream.Collectors;
/**
@@ -125,17 +124,11 @@ public class ScaleRuleServiceImpl implements
ScaleRuleService {
*/
@Override
public int update(final ScaleRuleDTO scaleRuleDTO) {
- final ScaleRuleDO before =
scaleRuleMapper.selectByPrimaryKey(scaleRuleDTO.getId());
final ScaleRuleDO after = ScaleRuleDO.buildScaleRuleDO(scaleRuleDTO);
int rows = scaleRuleMapper.updateByPrimaryKeySelective(after);
if (rows > 0) {
final ScaleRuleDO persisted =
scaleRuleMapper.selectByPrimaryKey(scaleRuleDTO.getId());
- runAfterCommit(() -> {
- if (Objects.nonNull(before) &&
!Objects.equals(before.getMetricName(), after.getMetricName())) {
-
scaleRuleCache.removeRulesFromCache(List.of(before.getMetricName()));
- }
- scaleRuleCache.addOrUpdateRuleToCache(persisted);
- });
+ runAfterCommit(() ->
scaleRuleCache.addOrUpdateRuleToCache(persisted));
}
return rows;
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCacheTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCacheTest.java
new file mode 100644
index 0000000000..4f6d223f12
--- /dev/null
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/scale/monitor/subject/cache/ScaleRuleCacheTest.java
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 org.apache.shenyu.admin.scale.monitor.subject.cache;
+
+import org.apache.shenyu.admin.model.entity.ScaleRuleDO;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests for the scaling rule cache.
+ */
+public final class ScaleRuleCacheTest {
+
+ @Test
+ public void testInitializeKeepsRulesWithSameMetric() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = ScaleRuleDO.builder()
+ .id("rule-a")
+ .metricName("cpu_usage")
+ .maximum("60")
+ .build();
+ final ScaleRuleDO second = ScaleRuleDO.builder()
+ .id("rule-b")
+ .metricName("cpu_usage")
+ .maximum("80")
+ .build();
+
+ cache.initialize(List.of(first, second));
+
+ assertEquals(2, cache.getAllRules().size());
+ }
+
+ @Test
+ public void testAddingRulesWithSameMetricKeepsBothRules() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = rule("a", "cpu_usage", 1);
+ final ScaleRuleDO second = rule("b", "cpu_usage", 2);
+
+ cache.addOrUpdateRuleToCache(first);
+ cache.addOrUpdateRuleToCache(second);
+
+ assertEquals(2, cache.getAllRules().size());
+ assertTrue(cache.getAllRules().containsAll(List.of(first, second)));
+ }
+
+ @Test
+ public void testUpdatingRuleReplacesOnlyItsOwnEntry() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = rule("a", "cpu_usage", 1);
+ final ScaleRuleDO second = rule("b", "cpu_usage", 2);
+ cache.initialize(List.of(first, second));
+ final ScaleRuleDO updated = rule("a", "cpu_usage", 3);
+
+ cache.addOrUpdateRuleToCache(updated);
+
+ assertEquals(2, cache.getAllRules().size());
+ assertTrue(cache.getAllRules().containsAll(List.of(updated, second)));
+ }
+
+ @Test
+ public void testRenamingRuleKeepsOtherRulesForOriginalMetric() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = rule("a", "cpu_usage", 1);
+ final ScaleRuleDO second = rule("b", "cpu_usage", 2);
+ cache.initialize(List.of(first, second));
+ final ScaleRuleDO renamed = rule("a", "memory_usage", 1);
+
+ cache.addOrUpdateRuleToCache(renamed);
+
+ assertEquals(2, cache.getAllRules().size());
+ assertSame(second, cache.getRuleByMetricName("cpu_usage"));
+ assertSame(renamed, cache.getRuleByMetricName("memory_usage"));
+ }
+
+ @Test
+ public void testDeletingRuleByIdKeepsOtherRulesForSameMetric() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = rule("a", "cpu_usage", 1);
+ final ScaleRuleDO second = rule("b", "cpu_usage", 2);
+ cache.initialize(List.of(first, second));
+
+ cache.removeRulesByIdsFromCache(List.of("b", "missing"));
+
+ assertEquals(List.of(first), cache.getAllRules());
+ }
+
+ @Test
+ public void testDeletingByMetricRemovesAllMatchingRules() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first = rule("a", "cpu_usage", 1);
+ final ScaleRuleDO second = rule("b", "cpu_usage", 2);
+ final ScaleRuleDO other = rule("c", "memory_usage", 1);
+ cache.initialize(List.of(first, second, other));
+
+ cache.removeRulesFromCache(List.of("cpu_usage"));
+
+ assertEquals(List.of(other), cache.getAllRules());
+ }
+
+ @Test
+ public void testInitializeReplacesPreviousRules() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ cache.initialize(List.of(rule("a", "cpu_usage", 1)));
+ final ScaleRuleDO replacement = rule("b", "memory_usage", 1);
+
+ cache.initialize(List.of(replacement));
+
+ assertEquals(List.of(replacement), cache.getAllRules());
+ }
+
+ @Test
+ public void testMetricLookupUsesPriorityAndStableIdOrder() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO expected = rule("a", "cpu_usage", 1);
+ cache.initialize(List.of(rule("c", "cpu_usage", 2), rule("b",
"cpu_usage", 1),
+ rule("d", "cpu_usage", null), expected));
+
+ assertSame(expected, cache.getRuleByMetricName("cpu_usage"));
+ assertNull(cache.getRuleByMetricName("missing"));
+ }
+
+ private ScaleRuleDO rule(final String id, final String metricName, final
Integer sort) {
+ return
ScaleRuleDO.builder().id(id).metricName(metricName).sort(sort).build();
+ }
+}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ScaleRuleServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ScaleRuleServiceTest.java
index a49815f7db..f96990c1d7 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ScaleRuleServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ScaleRuleServiceTest.java
@@ -34,8 +34,10 @@ import
org.springframework.transaction.support.TransactionSynchronizationManager
import org.springframework.transaction.support.TransactionSynchronizationUtils;
import java.sql.Timestamp;
+import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
@@ -103,13 +105,12 @@ public final class ScaleRuleServiceTest {
ScaleRuleDTO dto = new ScaleRuleDTO();
dto.setId("r");
dto.setMetricName("new");
-
when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(ScaleRuleDO.builder().id("r").metricName("old").build(),
- ScaleRuleDO.builder().id("r").metricName("new").build());
+
when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(ScaleRuleDO.builder().id("r").metricName("new").build());
when(scaleRuleMapper.updateByPrimaryKeySelective(any(ScaleRuleDO.class))).thenReturn(1);
TransactionSynchronizationManager.initSynchronization();
scaleRuleService.update(dto);
TransactionSynchronizationUtils.invokeAfterCommit(TransactionSynchronizationManager.getSynchronizations());
- verify(scaleRuleCache).removeRulesFromCache(any());
+ verify(scaleRuleCache, never()).removeRulesFromCache(any());
verify(scaleRuleCache).addOrUpdateRuleToCache(any(ScaleRuleDO.class));
TransactionSynchronizationManager.clearSynchronization();
when(scaleRuleMapper.delete(any())).thenReturn(1);
@@ -125,7 +126,7 @@ public final class ScaleRuleServiceTest {
ScaleRuleDTO update = new ScaleRuleDTO();
update.setId("r");
ScaleRuleDO persisted =
ScaleRuleDO.builder().id("r").metricName("metric").build();
- when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(null,
persisted);
+ when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(persisted);
when(scaleRuleMapper.updateByPrimaryKeySelective(any(ScaleRuleDO.class))).thenReturn(1);
assertEquals(1, scaleRuleService.createOrUpdate(update));
verify(scaleRuleMapper).insertSelective(any(ScaleRuleDO.class));
@@ -150,7 +151,6 @@ public final class ScaleRuleServiceTest {
public void testUpdateAndDeleteSkipCacheWhenMapperReturnsZero() {
ScaleRuleDTO dto = new ScaleRuleDTO();
dto.setId("r");
- when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(null);
when(scaleRuleMapper.updateByPrimaryKeySelective(any(ScaleRuleDO.class))).thenReturn(0);
assertEquals(0, scaleRuleService.update(dto));
when(scaleRuleMapper.delete(any())).thenReturn(0);
@@ -172,11 +172,10 @@ public final class ScaleRuleServiceTest {
}
@Test
- public void testUpdateWithoutPreviousRuleAndDeleteAfterCommit() {
+ public void testUpdateAndDeletePublishCacheChangesAfterCommit() {
ScaleRuleDTO dto = new ScaleRuleDTO();
dto.setId("r");
- when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(null,
- ScaleRuleDO.builder().id("r").metricName("metric").build());
+
when(scaleRuleMapper.selectByPrimaryKey("r")).thenReturn(ScaleRuleDO.builder().id("r").metricName("metric").build());
when(scaleRuleMapper.updateByPrimaryKeySelective(any(ScaleRuleDO.class))).thenReturn(1);
TransactionSynchronizationManager.initSynchronization();
scaleRuleService.update(dto);
@@ -191,4 +190,47 @@ public final class ScaleRuleServiceTest {
TransactionSynchronizationUtils.invokeAfterCommit(TransactionSynchronizationManager.getSynchronizations());
verify(scaleRuleCache).removeRulesByIdsFromCache(any());
}
+
+ @Test
+ public void testRenamePreservesOtherRulesWithSameMetricAfterCommit() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first =
ScaleRuleDO.builder().id("a").metricName("cpu_usage").build();
+ final ScaleRuleDO second =
ScaleRuleDO.builder().id("b").metricName("cpu_usage").build();
+ cache.initialize(List.of(first, second));
+ final ScaleRuleDO renamed =
ScaleRuleDO.builder().id("a").metricName("memory_usage").build();
+
when(scaleRuleMapper.updateByPrimaryKeySelective(any(ScaleRuleDO.class))).thenReturn(1);
+ when(scaleRuleMapper.selectByPrimaryKey("a")).thenReturn(renamed);
+ final ScaleRuleDTO update = new ScaleRuleDTO();
+ update.setId("a");
+ update.setMetricName("memory_usage");
+ final ScaleRuleService service = new
ScaleRuleServiceImpl(scaleRuleMapper, cache);
+ TransactionSynchronizationManager.initSynchronization();
+
+ service.update(update);
+
+ assertEquals(2, cache.getAllRules().size());
+ assertEquals("cpu_usage", first.getMetricName());
+ assertTrue(cache.getAllRules().contains(first));
+
TransactionSynchronizationUtils.invokeAfterCommit(TransactionSynchronizationManager.getSynchronizations());
+ assertEquals(2, cache.getAllRules().size());
+ assertTrue(cache.getAllRules().contains(renamed));
+ assertTrue(cache.getAllRules().contains(second));
+ }
+
+ @Test
+ public void testDeletePreservesOtherRulesWithSameMetricAfterCommit() {
+ final ScaleRuleCache cache = new ScaleRuleCache();
+ final ScaleRuleDO first =
ScaleRuleDO.builder().id("a").metricName("cpu_usage").build();
+ final ScaleRuleDO second =
ScaleRuleDO.builder().id("b").metricName("cpu_usage").build();
+ cache.initialize(List.of(first, second));
+ when(scaleRuleMapper.delete(List.of("a"))).thenReturn(1);
+ final ScaleRuleService service = new
ScaleRuleServiceImpl(scaleRuleMapper, cache);
+ TransactionSynchronizationManager.initSynchronization();
+
+ service.delete(List.of("a"));
+
+ assertEquals(2, cache.getAllRules().size());
+
TransactionSynchronizationUtils.invokeAfterCommit(TransactionSynchronizationManager.getSynchronizations());
+ assertEquals(List.of(second), cache.getAllRules());
+ }
}
