You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/07/27 08:20:07 UTC
[GitHub] [servicecomb-java-chassis] GuoYL123 opened a new pull request #1894: [SCB-2043] flow control bug fix
GuoYL123 opened a new pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894
(cherry picked from commit eed0e46cf448b0caab1b7211f9780bbd82f55a6b)
Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#discussion_r462717447
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +210,25 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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);
+ String strategyName) {
+ if (StringUtils.isEmpty(strategyName)) {
+ strategyName = "FixedWindow";
+ }
+ AbstractQpsStrategy strategy = null;
+ List<IStrategyFactory> strategyFactories = SPIServiceUtils
+ .getOrLoadSortedService(IStrategyFactory.class);
+ for (IStrategyFactory strategyFactory : strategyFactories) {
+ strategy = strategyFactory.getStrategy(strategyName);
+ if (strategy != null) {
break;
- case SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ }
+ }
+ if (strategy == null) {
+ throw new ServiceCombException("the qps strategy name is not exist , please check.");
Review comment:
exeception message is better to include `strategyName`
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/DefaultStrategyFactory.java
##########
@@ -14,27 +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;
-
-
- public static StrategyType parseStrategyType(String type) {
- if (StringUtils.isEmpty(type)) {
- return StrategyType.FixedWindow;
- }
+public class DefaultStrategyFactory implements IStrategyFactory {
- try {
- return StrategyType.valueOf(type.toUpperCase());
- } catch (IllegalArgumentException e) {
- return StrategyType.FixedWindow;
+ public AbstractQpsStrategy getStrategy(String strategyName) {
Review comment:
It's better to use `createStrategy`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] liubao68 merged pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
liubao68 merged pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#discussion_r462149918
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -17,20 +17,20 @@
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.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.netflix.config.DynamicProperty;
+import org.springframework.util.StringUtils;
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#discussion_r461293774
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
` new TokenBucketStrategy(globalConfigKey, limit, bucket);`
`new FixedWindowStrategy(globalConfigKey, limit);`
has different parameters.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
done
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -210,21 +209,17 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ AbstractQpsStrategy strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ List<AbstractQpsStrategy> customStrategy = SPIServiceUtils
+ .getOrLoadSortedService(AbstractQpsStrategy.class);
+ for (AbstractQpsStrategy abstractQpsStrategy : customStrategy) {
+ if (abstractQpsStrategy.name().equals(strategyKey)) {
Review comment:
already has local value 'strategy'
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -210,21 +209,17 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ AbstractQpsStrategy strategy = new FixedWindowStrategy(globalConfigKey, limit);
Review comment:
throw a Exception
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +208,21 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ String strategyName) {
Review comment:
when name is incorrect ， throw a Exception ?
` if (strategy == null) {
throw new ServiceCombException("the qps strategy name is not exist , please check.");
}`
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +208,21 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ String strategyName) {
Review comment:
when name is incorrect ， throw a Exception ， so it's no need to give a default value.
` if (strategy == null) {
throw new ServiceCombException("the qps strategy name is not exist , please check.");
}`
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +208,21 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ String strategyName) {
Review comment:
when name is incorrect ， throw a Exception ， so it's no need to give a default value.
` if (strategy == null) {
throw new ServiceCombException("the qps strategy name is not exist , please check.");
}`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
[](https://coveralls.io/builds/32424250)
Coverage increased (+0.02%) to 86.317% when pulling **3d9373dd00f21c318f77fb72d7fd77641ed2ed9b on GuoYL123:master** into **10381beef0ad6689612cea58c0e42ee51dafaf5b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
[](https://coveralls.io/builds/32415683)
Coverage increased (+0.03%) to 86.332% when pulling **f62dee43e5517b3dcf80848aa50c44e6d11a7132 on GuoYL123:master** into **845d763e5bab180285116ab5a0d6f4ea5f528728 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#discussion_r461262652
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
Maybe this code can be better refactored.
1. all strategies are SPI
2. any implementation have a name
3. choose the target strategy by configured name and default to 'FixedWindow'
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
Maybe this code can be better refactored.
1. all strategies are SPI
2. any implementation have a name
3. choose the target strategy by configured name and default to 'FixedWindow'
By doing this, the strategy enum and case statements can be removed.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
Hidden parameters in implementation and I think will work.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -211,6 +212,8 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
AbstractQpsStrategy strategy;
+ AbstractQpsStrategy customStrategy = SPIServiceUtils
Review comment:
Hidden parameters in implementation and I think will work. You can think of users will custom strategies like this, how will they do it?
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -210,21 +209,17 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ AbstractQpsStrategy strategy = new FixedWindowStrategy(globalConfigKey, limit);
Review comment:
strategyKey can have default value 'FixedWindow', wrong used defined value can throw exception. So here do not need new a default strategy.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -210,21 +209,17 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ AbstractQpsStrategy strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ List<AbstractQpsStrategy> customStrategy = SPIServiceUtils
+ .getOrLoadSortedService(AbstractQpsStrategy.class);
+ for (AbstractQpsStrategy abstractQpsStrategy : customStrategy) {
+ if (abstractQpsStrategy.name().equals(strategyKey)) {
+ strategy = abstractQpsStrategy;
Review comment:
Add a break is better.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -210,21 +209,17 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ AbstractQpsStrategy strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ List<AbstractQpsStrategy> customStrategy = SPIServiceUtils
+ .getOrLoadSortedService(AbstractQpsStrategy.class);
+ for (AbstractQpsStrategy abstractQpsStrategy : customStrategy) {
+ if (abstractQpsStrategy.name().equals(strategyKey)) {
Review comment:
strategyKey better to rename to 'strategy', it is not a key , but a value.
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +208,21 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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 SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ String strategyName) {
Review comment:
strategyName do not have a default value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#discussion_r462821368
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsControllerManager.java
##########
@@ -209,22 +210,25 @@ public QpsControllerManager setGlobalQpsStrategy(String globalLimitKey, String g
}
private AbstractQpsStrategy chooseStrategy(String globalConfigKey, Long limit, Long bucket,
- String strategyKey) {
- AbstractQpsStrategy strategy;
- 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);
+ String strategyName) {
+ if (StringUtils.isEmpty(strategyName)) {
+ strategyName = "FixedWindow";
+ }
+ AbstractQpsStrategy strategy = null;
+ List<IStrategyFactory> strategyFactories = SPIServiceUtils
+ .getOrLoadSortedService(IStrategyFactory.class);
+ for (IStrategyFactory strategyFactory : strategyFactories) {
+ strategy = strategyFactory.getStrategy(strategyName);
+ if (strategy != null) {
break;
- case SlidingWindow:
- default:
- strategy = new FixedWindowStrategy(globalConfigKey, limit);
+ }
+ }
+ if (strategy == null) {
+ throw new ServiceCombException("the qps strategy name is not exist , please check.");
Review comment:
done
##########
File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/strategy/DefaultStrategyFactory.java
##########
@@ -14,27 +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;
-
-
- public static StrategyType parseStrategyType(String type) {
- if (StringUtils.isEmpty(type)) {
- return StrategyType.FixedWindow;
- }
+public class DefaultStrategyFactory implements IStrategyFactory {
- try {
- return StrategyType.valueOf(type.toUpperCase());
- } catch (IllegalArgumentException e) {
- return StrategyType.FixedWindow;
+ public AbstractQpsStrategy getStrategy(String strategyName) {
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1894: [SCB-2043] flow control support custom / bug fix
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
[](https://coveralls.io/builds/32424620)
Coverage increased (+0.03%) to 86.333% when pulling **3d9373dd00f21c318f77fb72d7fd77641ed2ed9b on GuoYL123:master** into **10381beef0ad6689612cea58c0e42ee51dafaf5b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-java-chassis] coveralls commented on pull request #1894: [SCB-2043] flow control bug fix
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #1894:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1894#issuecomment-664256329
[](https://coveralls.io/builds/32340026)
Coverage increased (+0.01%) to 86.312% when pulling **55aa16f90829c52e8ec0b0cea557d448656d7510 on GuoYL123:master** into **88c24e95746cb143edf2405696004d9946aa04f0 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org