You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2022/06/23 09:04:14 UTC

[servicecomb-java-chassis] branch master updated: [SCB-2564]governance policy sort by key so that we can configure policy priority (#3115)

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 6d7ea2d86 [SCB-2564]governance policy sort by key so that we can configure policy priority (#3115)
6d7ea2d86 is described below

commit 6d7ea2d861d642906cede2d5edcb8407d2cc5b12
Author: liubao68 <bi...@qq.com>
AuthorDate: Thu Jun 23 17:04:09 2022 +0800

    [SCB-2564]governance policy sort by key so that we can configure policy priority (#3115)
---
 .../servicecomb/governance/MatchersManager.java    | 15 ++++---
 .../governance/policy/AbstractPolicy.java          | 20 ++++++++--
 .../governance/BulkheadHandlerTest.java            | 46 ++++++++++++++++++++++
 .../governance/GovernancePropertiesTest.java       | 22 +++++------
 governance/src/test/resources/application.yaml     | 10 +++++
 5 files changed, 93 insertions(+), 20 deletions(-)

diff --git a/governance/src/main/java/org/apache/servicecomb/governance/MatchersManager.java b/governance/src/main/java/org/apache/servicecomb/governance/MatchersManager.java
index 1bb9a346e..4e9685aaa 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/MatchersManager.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/MatchersManager.java
@@ -16,8 +16,9 @@
  */
 package org.apache.servicecomb.governance;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 
 import org.apache.servicecomb.governance.marker.GovernanceRequest;
 import org.apache.servicecomb.governance.policy.AbstractPolicy;
@@ -36,15 +37,17 @@ public class MatchersManager {
   public <T extends AbstractPolicy> T match(GovernanceRequest request, Map<String, T> policies) {
     Map<String, Boolean> calculatedMatches = invocationContext.getCalculatedMatches();
 
-    for (Entry<String, T> entry : policies.entrySet()) {
-      T policy = entry.getValue();
+    List<T> sortPolicies = new ArrayList<>(policies.size());
+    sortPolicies.addAll(policies.values());
+    sortPolicies.sort(T::compareTo);
 
-      if (calculatedMatches.containsKey(entry.getKey())) {
+    for (T policy : sortPolicies) {
+      if (calculatedMatches.containsKey(policy.getName())) {
         return policy;
       }
 
-      boolean keyMatch = matchersService.checkMatch(request, entry.getKey());
-      invocationContext.addMatch(entry.getKey(), keyMatch);
+      boolean keyMatch = matchersService.checkMatch(request, policy.getName());
+      invocationContext.addMatch(policy.getName(), keyMatch);
       if (keyMatch) {
         return policy;
       }
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
index 5fe427d80..c1638bde0 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
@@ -16,19 +16,33 @@
  */
 package org.apache.servicecomb.governance.policy;
 
+import java.time.Duration;
+
 import org.apache.commons.lang3.StringUtils;
 import org.apache.servicecomb.governance.entity.Configurable;
 import org.apache.servicecomb.governance.utils.GovernanceUtils;
 
-import java.time.Duration;
-
-public abstract class AbstractPolicy extends Configurable {
+public abstract class AbstractPolicy extends Configurable implements Comparable<AbstractPolicy> {
+  protected int order = 0;
 
   @Override
   public boolean isValid() {
     return !StringUtils.isEmpty(name);
   }
 
+  @Override
+  public int compareTo(AbstractPolicy o) {
+    return this.order - o.order;
+  }
+
+  public int getOrder() {
+    return order;
+  }
+
+  public void setOrder(int order) {
+    this.order = order;
+  }
+
   private Duration parseToDuration(String time, Duration defaultValue) {
     if (StringUtils.isEmpty(time)) {
       return defaultValue;
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/BulkheadHandlerTest.java b/governance/src/test/java/org/apache/servicecomb/governance/BulkheadHandlerTest.java
new file mode 100644
index 000000000..d08541b51
--- /dev/null
+++ b/governance/src/test/java/org/apache/servicecomb/governance/BulkheadHandlerTest.java
@@ -0,0 +1,46 @@
+/*
+ * 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.servicecomb.governance;
+
+import org.apache.servicecomb.governance.handler.BulkheadHandler;
+import org.apache.servicecomb.governance.marker.GovernanceRequest;
+import org.apache.servicecomb.governance.policy.BulkheadPolicy;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ContextConfiguration;
+
+@SpringBootTest
+@ContextConfiguration(classes = {GovernanceConfiguration.class, MockConfiguration.class})
+public class BulkheadHandlerTest {
+  private BulkheadHandler bulkheadHandler;
+
+  @Autowired
+  public void setInstanceIsolationHandler(BulkheadHandler bulkheadHandler) {
+    this.bulkheadHandler = bulkheadHandler;
+  }
+
+  @Test
+  public void testMatchPriorityPolicy() {
+    GovernanceRequest request = new GovernanceRequest();
+    request.setUri("/bulkhead");
+    BulkheadPolicy policy = bulkheadHandler.matchPolicy(request);
+    Assertions.assertEquals("demo-bulkhead-priority", policy.getName());
+  }
+}
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java b/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
index aaec8e19a..f234dee0a 100644
--- a/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
@@ -159,7 +159,7 @@ public class GovernancePropertiesTest {
   @Test
   public void test_match_properties_successfully_loaded() {
     Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
-    Assertions.assertEquals(5, markers.size());
+    Assertions.assertEquals(6, markers.size());
     TrafficMarker demoRateLimiting = markers.get("demo-rateLimiting");
     List<Matcher> matchers = demoRateLimiting.getMatches();
     Assertions.assertEquals(1, matchers.size());
@@ -177,17 +177,17 @@ public class GovernancePropertiesTest {
   @Test
   public void test_match_properties_delete() {
     Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
-    Assertions.assertEquals(5, markers.size());
+    Assertions.assertEquals(6, markers.size());
     dynamicValues.put("servicecomb.matchGroup.test", "matches:\n"
         + "  - apiPath:\n"
         + "      exact: \"/hello2\"\n"
         + "    name: match0");
     GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
     markers = matchProperties.getParsedEntity();
-    Assertions.assertEquals(6, markers.size());
+    Assertions.assertEquals(7, markers.size());
     tearDown();
     markers = matchProperties.getParsedEntity();
-    Assertions.assertEquals(5, markers.size());
+    Assertions.assertEquals(6, markers.size());
   }
 
   @Test
@@ -204,7 +204,7 @@ public class GovernancePropertiesTest {
     GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
 
     Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
-    Assertions.assertEquals(6, markers.size());
+    Assertions.assertEquals(7, markers.size());
     TrafficMarker demoRateLimiting = markers.get("demo-rateLimiting");
     List<Matcher> matchers = demoRateLimiting.getMatches();
     Assertions.assertEquals(1, matchers.size());
@@ -230,13 +230,13 @@ public class GovernancePropertiesTest {
     GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
 
     Map<String, BulkheadPolicy> policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(2, policies.size());
+    Assertions.assertEquals(3, policies.size());
     BulkheadPolicy policy = policies.get("demo-bulkhead");
     Assertions.assertEquals(2, policy.getMaxConcurrentCalls());
     Assertions.assertEquals(2000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
 
     policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(2, policies.size());
+    Assertions.assertEquals(3, policies.size());
     policy = policies.get("bulkhead1");
     Assertions.assertEquals(3, policy.getMaxConcurrentCalls());
     Assertions.assertEquals(3000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
@@ -258,7 +258,7 @@ public class GovernancePropertiesTest {
     GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
 
     Map<String, BulkheadPolicy> policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(4, policies.size());
+    Assertions.assertEquals(5, policies.size());
     BulkheadPolicy policy = policies.get("test-bulkhead1");
     Assertions.assertEquals(0, policy.getMaxConcurrentCalls());
     Assertions.assertEquals(2000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
@@ -277,7 +277,7 @@ public class GovernancePropertiesTest {
   @Test
   public void test_bulkhead_properties_successfully_loaded() {
     Map<String, BulkheadPolicy> policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(1, policies.size());
+    Assertions.assertEquals(2, policies.size());
     BulkheadPolicy policy = policies.get("demo-bulkhead");
     Assertions.assertEquals(1, policy.getMaxConcurrentCalls());
     Assertions.assertEquals(3000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
@@ -370,12 +370,12 @@ public class GovernancePropertiesTest {
     GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
 
     Map<String, BulkheadPolicy> policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(3, policies.size());
+    Assertions.assertEquals(4, policies.size());
     BulkheadPolicy policy = policies.get("test1");
     Assertions.assertEquals(2000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
 
     policies = bulkheadProperties.getParsedEntity();
-    Assertions.assertEquals(3, policies.size());
+    Assertions.assertEquals(4, policies.size());
     policy = policies.get("test2");
     Assertions.assertEquals(60000, Duration.parse(policy.getMaxWaitDuration()).toMillis());
   }
diff --git a/governance/src/test/resources/application.yaml b/governance/src/test/resources/application.yaml
index b76e489eb..28221657f 100644
--- a/governance/src/test/resources/application.yaml
+++ b/governance/src/test/resources/application.yaml
@@ -45,6 +45,10 @@ servicecomb:
           headers:
             x-token:
               exact: token
+    demo-bulkhead-priority: |
+      matches:
+        - apiPath:
+          exact: "/bulkhead"
     demo-bulkhead-other: |
       matches:
         - apiPath:
@@ -74,6 +78,12 @@ servicecomb:
       minimumNumberOfCalls: -1
   bulkhead:
     demo-bulkhead: |
+      order: 1
+      maxConcurrentCalls: 1
+      maxWaitDuration: 3000
+      services: myself:1.0
+    demo-bulkhead-priority: |
+      order: 0
       maxConcurrentCalls: 1
       maxWaitDuration: 3000
       services: myself:1.0