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 2020/07/30 11:43:00 UTC

[servicecomb-java-chassis] 03/04: [SCB-2043] refactory as comment

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

commit 84eb8bebfed0547e5b971af9f46c864598b12273
Author: GuoYL <gy...@gmail.com>
AuthorDate: Tue Jul 28 17:26:06 2020 +0800

    [SCB-2043] refactory as comment
---
 .../servicecomb/qps/QpsControllerManager.java      | 45 +++++++++++-----------
 .../org/apache/servicecomb/qps/QpsStrategy.java    |  2 +
 .../qps/strategy/AbstractQpsStrategy.java          | 14 +++----
 ...rategyType.java => DefaultStrategyFactory.java} | 30 ++++++---------
 .../qps/strategy/FixedWindowStrategy.java          | 10 +++--
 .../IStrategyFactory.java}                         |  6 +--
 .../qps/strategy/LeakyBucketStrategy.java          | 14 +++----
 .../qps/strategy/TokenBucketStrategy.java          |  7 +++-
 ...pache.servicecomb.qps.strategy.IStrategyFactory | 18 +++++++++
 .../qps/TestConsumerQpsFlowControlHandler.java     | 12 ++++--
 .../qps/TestProviderQpsFlowControlHandler.java     | 14 +++++--
 .../apache/servicecomb/qps/TestQpsStrategy.java    |  8 +++-
 12 files changed, 103 insertions(+), 77 deletions(-)

diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
index 606b642..a287ca2 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
@@ -17,21 +17,21 @@
 
 package org.apache.servicecomb.qps;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
+import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
 import org.apache.servicecomb.qps.strategy.AbstractQpsStrategy;
-import org.apache.servicecomb.qps.strategy.FixedWindowStrategy;
-import org.apache.servicecomb.qps.strategy.LeakyBucketStrategy;
-import org.apache.servicecomb.qps.strategy.StrategyType;
-import org.apache.servicecomb.qps.strategy.TokenBucketStrategy;
+import org.apache.servicecomb.qps.strategy.IStrategyFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.netflix.config.DynamicProperty;
+import org.apache.commons.lang3.StringUtils;
 
 public class QpsControllerManager {
   private static final Logger LOGGER = LoggerFactory.getLogger(QpsControllerManager.class);
@@ -210,27 +210,26 @@ public class QpsControllerManager {
   }
 
   private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
-      String strategyKey) {
-    AbstractQpsStrategy strategy;
-    AbstractQpsStrategy customStrategy = SPIServiceUtils
-        .getTargetService(AbstractQpsStrategy.class);
-    switch (StrategyType.parseStrategyType(strategyKey)) {
-      case FixedWindow:
-        strategy = new FixedWindowStrategy(globalConfigKey, limit);
-        break;
-      case LeakyBucket:
-        strategy = new LeakyBucketStrategy(globalConfigKey, limit);
-        break;
-      case TokenBucket:
-        strategy = new TokenBucketStrategy(globalConfigKey, limit, bucket);
-        break;
-      case Custom:
-        strategy = customStrategy;
+      String strategyName) {
+    if (StringUtils.isEmpty(strategyName)) {
+      strategyName = "FixedWindow";
+    }
+    AbstractQpsStrategy strategy = null;
+    List<IStrategyFactory> strategyFactories = SPIServiceUtils
+        .getOrLoadSortedService(IStrategyFactory.class);
+    for (IStrategyFactory strategyFactory : strategyFactories) {
+      strategy = strategyFactory.createStrategy(strategyName);
+      if (strategy != null) {
         break;
-      case SlidingWindow:
-      default:
-        strategy = new FixedWindowStrategy(globalConfigKey, limit);
+      }
+    }
+    if (strategy == null) {
+      throw new ServiceCombException(
+          "the qps strategy name " + strategyName + " is not exist , please check.");
     }
+    strategy.setKey(globalConfigKey);
+    strategy.setQpsLimit(limit);
+    strategy.setBucketLimit(bucket);
     return strategy;
   }
 
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
index 606d7b8..8a712e3 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
@@ -20,4 +20,6 @@ package org.apache.servicecomb.qps;
 public interface QpsStrategy {
 
   boolean isLimitNewRequest();
+
+  String name();
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java
index 4fc0ed8..65d36aa 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/AbstractQpsStrategy.java
@@ -20,7 +20,7 @@ package org.apache.servicecomb.qps.strategy;
 import org.apache.servicecomb.qps.QpsStrategy;
 
 
-public class AbstractQpsStrategy implements QpsStrategy {
+public abstract class AbstractQpsStrategy implements QpsStrategy {
 
   private Long qpsLimit;
 
@@ -28,11 +28,6 @@ public class AbstractQpsStrategy implements QpsStrategy {
 
   private String key;
 
-  public AbstractQpsStrategy(Long qpsLimit, String key) {
-    this.qpsLimit = qpsLimit;
-    this.key = key;
-  }
-
   public Long getBucketLimit() {
     return bucketLimit;
   }
@@ -42,9 +37,10 @@ public class AbstractQpsStrategy implements QpsStrategy {
   }
 
   @Override
-  public boolean isLimitNewRequest() {
-    return true;
-  }
+  public abstract boolean isLimitNewRequest();
+
+  @Override
+  public abstract String name();
 
   public void setQpsLimit(Long qpsLimit) {
     this.qpsLimit = qpsLimit;
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/StrategyType.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/DefaultStrategyFactory.java
similarity index 66%
rename from handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/StrategyType.java
rename to handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/DefaultStrategyFactory.java
index 7412b0a..79037f7 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/StrategyType.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/DefaultStrategyFactory.java
@@ -14,28 +14,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.servicecomb.qps.strategy;
 
-import org.apache.commons.lang3.StringUtils;
-
-public enum StrategyType {
-  TokenBucket,
-  LeakyBucket,
-  FixedWindow,
-  SlidingWindow,
-  Custom;
-
-
-  public static StrategyType parseStrategyType(String type) {
-    if (StringUtils.isEmpty(type)) {
-      return StrategyType.FixedWindow;
-    }
+public class DefaultStrategyFactory implements IStrategyFactory {
 
-    try {
-      return StrategyType.valueOf(type);
-    } catch (IllegalArgumentException e) {
-      return StrategyType.FixedWindow;
+  public AbstractQpsStrategy createStrategy(String strategyName) {
+    switch (strategyName) {
+      case "TokenBucket":
+        return new TokenBucketStrategy();
+      case "LeakyBucket":
+        return new LeakyBucketStrategy();
+      case "FixedWindow":
+        return new FixedWindowStrategy();
+      default:
+        return null;
     }
   }
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java
index 74bb9b5..3c5fe63 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/FixedWindowStrategy.java
@@ -31,10 +31,7 @@ public class FixedWindowStrategy extends AbstractQpsStrategy {
 
   private static final int CYCLE_LENGTH = 1000;
 
-  public FixedWindowStrategy(String key, Long qpsLimit) {
-    super(qpsLimit, key);
-    this.msCycleBegin = System.currentTimeMillis();
-  }
+  private static final String STRATEGY_NAME = "FixedWindow";
 
   // return true means new request need to be rejected
   public boolean isLimitNewRequest() {
@@ -54,4 +51,9 @@ public class FixedWindowStrategy extends AbstractQpsStrategy {
     // It is possible that operation level updated to null,but schema level or microservice level does not updated
     return newCount - lastRequestCount >= this.getQpsLimit();
   }
+
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
+  }
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/IStrategyFactory.java
similarity index 85%
copy from handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
copy to handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/IStrategyFactory.java
index 606d7b8..bebe8ce 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/IStrategyFactory.java
@@ -15,9 +15,9 @@
  * limitations under the License.
  */
 
-package org.apache.servicecomb.qps;
+package org.apache.servicecomb.qps.strategy;
 
-public interface QpsStrategy {
+public interface IStrategyFactory {
 
-  boolean isLimitNewRequest();
+  AbstractQpsStrategy createStrategy(String strategyName);
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java
index b2f4f3e..d6a65ee 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/LeakyBucketStrategy.java
@@ -34,15 +34,7 @@ public class LeakyBucketStrategy extends AbstractQpsStrategy {
 
   private long remainder = 0;
 
-  public LeakyBucketStrategy(String key, Long qpsLimit) {
-    super(qpsLimit, key);
-    this.setBucketLimit(qpsLimit);
-  }
-
-  public LeakyBucketStrategy(String key, Long qpsLimit, Long bucketLimit) {
-    super(qpsLimit, key);
-    this.setBucketLimit(bucketLimit);
-  }
+  private static final String STRATEGY_NAME = "LeakyBucket";
 
   @Override
   public boolean isLimitNewRequest() {
@@ -72,4 +64,8 @@ public class LeakyBucketStrategy extends AbstractQpsStrategy {
     return true;
   }
 
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
+  }
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java
index 68f3334..082906f 100644
--- a/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/TokenBucketStrategy.java
@@ -19,7 +19,10 @@ package org.apache.servicecomb.qps.strategy;
 
 public class TokenBucketStrategy extends LeakyBucketStrategy {
 
-  public TokenBucketStrategy(String key, Long qpsLimit, Long bucketLimit) {
-    super(key, qpsLimit, bucketLimit);
+  private static final String STRATEGY_NAME = "TokenBucket";
+
+  @Override
+  public String name() {
+    return STRATEGY_NAME;
   }
 }
diff --git a/handlers/handler-flowcontrol-qps/src/main/resources/META-INF/services/org.apache.servicecomb.qps.strategy.IStrategyFactory b/handlers/handler-flowcontrol-qps/src/main/resources/META-INF/services/org.apache.servicecomb.qps.strategy.IStrategyFactory
new file mode 100644
index 0000000..32f53fa
--- /dev/null
+++ b/handlers/handler-flowcontrol-qps/src/main/resources/META-INF/services/org.apache.servicecomb.qps.strategy.IStrategyFactory
@@ -0,0 +1,18 @@
+#
+# 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.
+#
+
+org.apache.servicecomb.qps.strategy.DefaultStrategyFactory
diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
index 619ef76..1cda2cf 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
@@ -70,7 +70,9 @@ public class TestConsumerQpsFlowControlHandler {
 
   @Test
   public void testQpsController() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -80,7 +82,9 @@ public class TestConsumerQpsFlowControlHandler {
   @Test
   public void testHandle() throws Exception {
     String key = "svc.schema.opr";
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("key", 12L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("key");
+    qpsStrategy.setQpsLimit(12L);
     Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
     Mockito.when(operationMeta.getSchemaQualifiedName()).thenReturn("schema.opr");
     Mockito.when(invocation.getSchemaId()).thenReturn("schema");
@@ -113,7 +117,9 @@ public class TestConsumerQpsFlowControlHandler {
   @Test
   public void testHandleIsLimitNewRequestAsFalse() throws Exception {
     String key = "service.schema.id";
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("service", 12L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("service");
+    qpsStrategy.setQpsLimit(12L);
     Mockito.when(invocation.getMicroserviceName()).thenReturn("service");
     Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
 
diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
index 6182007..39cc1b9 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
@@ -99,7 +99,9 @@ public class TestProviderQpsFlowControlHandler {
 
   @Test
   public void testQpsController() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     assertFalse(qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -144,7 +146,10 @@ public class TestProviderQpsFlowControlHandler {
     new MockUp<QpsControllerManager>() {
       @Mock
       protected QpsStrategy create(String qualifiedNameKey) {
-        return new FixedWindowStrategy(qualifiedNameKey, 1L);
+        AbstractQpsStrategy strategy = new FixedWindowStrategy();
+        strategy.setKey(qualifiedNameKey);
+        strategy.setQpsLimit(1L);
+        return strategy;
       }
     };
 
@@ -171,7 +176,10 @@ public class TestProviderQpsFlowControlHandler {
     new MockUp<QpsControllerManager>() {
       @Mock
       protected QpsStrategy create(String qualifiedNameKey) {
-        return new FixedWindowStrategy(qualifiedNameKey, 1L);
+        AbstractQpsStrategy strategy = new FixedWindowStrategy();
+        strategy.setKey(qualifiedNameKey);
+        strategy.setQpsLimit(1L);
+        return strategy;
       }
     };
     handler.handle(invocation, asyncResp);
diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java
index 75beb71..04c6f02 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestQpsStrategy.java
@@ -31,7 +31,9 @@ public class TestQpsStrategy {
 
   @Test
   public void testFixedWindowStrategy() {
-    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy("abc", 100L);
+    AbstractQpsStrategy qpsStrategy = new FixedWindowStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);
@@ -41,7 +43,9 @@ public class TestQpsStrategy {
 
   @Test
   public void testLeakyBucketStrategy() {
-    LeakyBucketStrategy qpsStrategy = new LeakyBucketStrategy("abc", 100L);
+    LeakyBucketStrategy qpsStrategy = new LeakyBucketStrategy();
+    qpsStrategy.setKey("abc");
+    qpsStrategy.setQpsLimit(100L);
     Assert.assertEquals(false, qpsStrategy.isLimitNewRequest());
 
     qpsStrategy.setQpsLimit(1L);