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 <gylg...@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);