[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16713554#comment-16713554 ] ASF GitHub Bot commented on SCB-1056: - coveralls commented on issue #1026: [SCB-1056] put provider QPS flow control in front, for highway transport URL: https://github.com/apache/servicecomb-java-chassis/pull/1026#issuecomment-445432590 [](https://coveralls.io/builds/20485156) Coverage decreased (-0.006%) to 86.737% when pulling **9507896155cef8bb2ab3dc233ad36250c6c3f73d on yhs0092:enhance_QPS_limit_highway** into **374f5e21c3a6b960242fc68b5bfe391498aac6da on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > Fix For: java-chassis-1.2.0 > > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16711228#comment-16711228
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 closed pull request #1026: [SCB-1056] put provider QPS flow control in
front, for highway transport
URL: https://github.com/apache/servicecomb-java-chassis/pull/1026
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
index 20e8e0828..32bfb67b9 100644
---
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
+++
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
@@ -20,12 +20,14 @@
import java.util.Map;
import javax.ws.rs.core.Response.Status;
+import javax.xml.ws.Holder;
import org.apache.servicecomb.codec.protobuf.definition.OperationProtobuf;
import org.apache.servicecomb.codec.protobuf.definition.ProtobufManager;
import org.apache.servicecomb.codec.protobuf.utils.WrapSchema;
import org.apache.servicecomb.core.Const;
import org.apache.servicecomb.core.Endpoint;
+import org.apache.servicecomb.core.Handler;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.SCBEngine;
import org.apache.servicecomb.core.definition.MicroserviceMeta;
@@ -181,9 +183,38 @@ public void execute() {
null);
invocation.onStart(null, start);
invocation.getInvocationStageTrace().startSchedule();
- operationMeta.getExecutor().execute(() -> runInExecutor());
+
+ // copied from HighwayCodec#decodeRequest()
+ // for temporary qps enhance purpose, we'll remove it when handler
mechanism is refactored
+ invocation.mergeContext(header.getContext());
+
+ Holder qpsFlowControlReject =
checkQpsFlowControl(operationMeta);
+ if (qpsFlowControlReject.value) {
+return;
+ }
+
+ operationMeta.getExecutor().execute(this::runInExecutor);
} catch (IllegalStateException e) {
sendResponse(header.getContext(), Response.providerFailResp(e));
}
}
+
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+@SuppressWarnings("deprecation")
+Handler providerQpsFlowControlHandler =
operationMeta.getProviderQpsFlowControlHandler();
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ sendResponse(header.getContext(), response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendResponse(header.getContext(), Response.providerFailResp(e));
+ }
+}
+return qpsFlowControlReject;
+ }
}
diff --git
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayCodec.java
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayCodec.java
index a9932e2a1..ad3feddc8 100644
---
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayCodec.java
+++
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayCodec.java
@@ -37,7 +37,6 @@
import org.apache.servicecomb.serviceregistry.ServiceRegistry;
import org.apache.servicecomb.serviceregistry.registry.ServiceRegistryFactory;
import org.apache.servicecomb.swagger.invocation.Response;
-import org.apache.servicecomb.swagger.invocation.context.InvocationContext;
import org.apache.servicecomb.transport.highway.message.RequestHeader;
import org.apache.servicecomb.transport.highway.message.ResponseHeader;
import org.junit.After;
@@ -81,7 +80,7 @@ public static void setupClass() {
}
@Before
- public void setUp() throws Exception {
+ public void setUp() {
ServiceRegistry serviceRegistry = ServiceRegistryFactory.createLocal();
serviceRegistry.init();
RegistryUtils.setServiceRegistry(serviceRegistry);
@@ -106,7 +105,7 @@ public void setUp() throws Exception {
}
@After
- public void tearDown() throws Exception {
+ public void tearDown() {
header = null;
diff --git
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayServerInvoke.java
b/transports/transport-highway/src/test/j
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16711199#comment-16711199 ] ASF GitHub Bot commented on SCB-1056: - yhs0092 opened a new pull request #1026: [SCB-1056] put provider QPS flow control in front, for highway transport URL: https://github.com/apache/servicecomb-java-chassis/pull/1026 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` 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). --- See details in [SCB-1056](https://issues.apache.org/jira/browse/SCB-1056), support this enhancement in highway transport. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > Fix For: java-chassis-1.2.0 > > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16708727#comment-16708727
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 closed pull request #1017: [SCB-1056] put provider flow control logic
in front
URL: https://github.com/apache/servicecomb-java-chassis/pull/1017
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
index 932aef4c3..d2b08aa05 100644
---
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
+++
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
@@ -26,6 +26,7 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response.Status;
+import javax.xml.ws.Holder;
import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor;
@@ -36,6 +37,7 @@
import org.apache.servicecomb.common.rest.locator.OperationLocator;
import org.apache.servicecomb.common.rest.locator.ServicePathManager;
import org.apache.servicecomb.core.Const;
+import org.apache.servicecomb.core.Handler;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.definition.MicroserviceMeta;
import org.apache.servicecomb.core.definition.OperationMeta;
@@ -126,6 +128,19 @@ protected void scheduleInvocation() {
invocation.getInvocationStageTrace().startSchedule();
OperationMeta operationMeta = restOperationMeta.getOperationMeta();
+try {
+ this.setContext();
+} catch (Exception e) {
+ LOGGER.error("failed to set invocation context", e);
+ sendFailResponse(e);
+ return;
+}
+
+Holder qpsFlowControlReject = checkQpsFlowControl(operationMeta);
+if (qpsFlowControlReject.value) {
+ return;
+}
+
operationMeta.getExecutor().execute(() -> {
synchronized (this.requestEx) {
try {
@@ -150,6 +165,26 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+@SuppressWarnings("deprecation")
+Handler providerQpsFlowControlHandler =
operationMeta.getProviderQpsFlowControlHandler();
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
+ }
+
private boolean isInQueueTimeout() {
return System.nanoTime() - invocation.getInvocationStageTrace().getStart()
>
CommonRestConfig.getRequestWaitInPoolTimeout() * 1_000_000;
@@ -183,7 +218,6 @@ public void invoke() {
protected Response prepareInvoke() throws Throwable {
this.initProduceProcessor();
-this.setContext();
invocation.getHandlerContext().put(RestConst.REST_REQUEST, requestEx);
invocation.getInvocationStageTrace().startServerFiltersRequest();
@@ -201,9 +235,7 @@ protected Response prepareInvoke() throws Throwable {
protected void doInvoke() throws Throwable {
invocation.getInvocationStageTrace().startHandlersRequest();
-invocation.next(resp -> {
- sendResponseQuietly(resp);
-});
+invocation.next(resp -> sendResponseQuietly(resp));
}
public void sendFailResponse(Throwable throwable) {
diff --git
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
index faf37a539..5a8c85962 100644
---
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
+++
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
@@ -17,6 +17,8 @@
package org.apache.servicecomb.common.rest;
+import static org.junit.Assert.assertEquals;
+
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -49,10 +51,11 @@
import org.apache.servicecomb.foundation.common.utils.JsonUtils;
import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
import org.a
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16708301#comment-16708301
]
ASF GitHub Bot commented on SCB-1056:
-
yhs0092 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238551098
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
+ }
+
+ @VisibleForTesting
+ Handler findQpsFlowControlHandler(OperationMeta operationMeta) {
Review comment:
Fixed. Please review it again.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16708299#comment-16708299 ] ASF GitHub Bot commented on SCB-1056: - yhs0092 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443998671 @liubao68 https://issues.apache.org/jira/browse/SCB-1064 New issue has been created. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16706791#comment-16706791
]
ASF GitHub Bot commented on SCB-1056:
-
wujimin commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238167366
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
+ }
+
+ @VisibleForTesting
+ Handler findQpsFlowControlHandler(OperationMeta operationMeta) {
Review comment:
it's not a good idea to find each time.
we can init and save it in OperationMeta, and mark the new field to be
temporary， only for internal usage
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16706636#comment-16706636 ] ASF GitHub Bot commented on SCB-1056: - liubao68 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443582182 Can you create a new issue to tracing the improvement? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16706588#comment-16706588 ] ASF GitHub Bot commented on SCB-1056: - yhs0092 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443566378 @liubao68 As wujimin says, a new handler mechanism should be designed to replace the current handler and filter mechanism. So I think there may be no need to change too much on current code. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705801#comment-16705801 ] ASF GitHub Bot commented on SCB-1056: - liubao68 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443423094 Maybe you can make QpsController as common module, and invoke the api in handler & AbstractInvocation. So that low controll in handler logic can remain the same, it's not that bad performance, or we can remove this handler in future. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705805#comment-16705805 ] ASF GitHub Bot commented on SCB-1056: - wujimin edited a comment on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443423422 @liubao68 i want to redesign mechanism about unify handler and filter and schedule executor, so that developers can arrange their logics in or not in eventloop, not hard code by framework current handlers/filters and executor scheduler can be wrap to the unified mechanism This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705804#comment-16705804 ] ASF GitHub Bot commented on SCB-1056: - wujimin commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443423422 @liubao68 i want to redesign mechanism about unify handler and filter and schedule executor, so that developers can arrange their logics in or not in eventloop, not hard code by framework This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705802#comment-16705802 ] ASF GitHub Bot commented on SCB-1056: - liubao68 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443423174 What do you mean by "a full refactoring design for handler mechanism" ? Optionally put handlers executed in event loop thread? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705798#comment-16705798
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238061857
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
Review comment:
I got it. Do you think it's better to give an option to turn on
ProviderQpsFlowControlHandler in AbstractRestInvocation, and do not change the
execution flow of handler? That is, when users do want
ProviderQpsFlowControlHandler executed in AbstractRestInvocation, they must
configure option to true, and remove handler configuration in chain.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705795#comment-16705795
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238062026
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
Review comment:
I mean "qpsFlowControlReject.value = true;" may executed after this method
return because "handle" is executed async. Although your code works as you
expected in this situation. It's not good code.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705792#comment-16705792
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238061857
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
Review comment:
I got it. Do you think it's better to give an option to turn on
ProviderQpsFlowControlHandler in AbstractRestInvocation, and do not change the
execution flow of handler? That is, when users do want
ProviderQpsFlowControlHandler executed in AbstractRestInvocation, they must
configure turn the option to true, and remove handler configuration in chain.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705610#comment-16705610 ] ASF GitHub Bot commented on SCB-1056: - yhs0092 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443396967 I think a better solution is to refactor the handler mechanism and let the handler execution position more flexiable. The PR just provides a temporary solution. And I don't use the `ProviderQpsFlowControlHandler` like `UiDispathcer` using `LoadBalanceHandler`, because I want to avoid importing extra maven dependency. The better solution may require a full refactoring design for handler mechanism. This should be tracked in a new issue. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705602#comment-16705602
]
ASF GitHub Bot commented on SCB-1056:
-
yhs0092 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238050682
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
Review comment:
I don't quite understand this statement.
Do you mean in line 175
```
providerQpsFlowControlHandler.handle(invocation, response -> {
```
the `response` may be null?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705600#comment-16705600
]
ASF GitHub Bot commented on SCB-1056:
-
yhs0092 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r238050519
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
Review comment:
No, `ProviderQpsFlowControlHandler` has been modified in this PR, too. Now
`ProviderQpsFlowControlHandler` checks whether it is in the handler chain. If
it finds it's in the handler chain, the `Invocation#next()` method will not be
invoked.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16704163#comment-16704163 ] ASF GitHub Bot commented on SCB-1056: - liubao68 edited a comment on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443069310 I don't think change the handler execution sequence is a good idea. I got another solution for using handler logic alone. Here is an example : https://github.com/huaweicse/cse-java-chassis-samples/blob/master/porter/gateway-service/src/main/java/com/huawei/cse/porter/gateway/UiDispatcher.java I used LoadBalancer handler here to do loadbalance and isolation logic for web page request dispatching I suggest add a configuration, e.g. servicecomb.flowcontrol.transport.enable to make flow controll logic executed before handler, and is a standalone logic, and you can resuse the code of handler. And the execution will not influence any handler logic(that's executed one by one, and if you configured flowcontrol handler, it will also be executed, that's twice execution). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16704162#comment-16704162 ] ASF GitHub Bot commented on SCB-1056: - liubao68 commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-443069310 I don't think change the handler execution sequence is a good idea. I got another solution for using handler logic alone. Here is an example : https://github.com/huaweicse/cse-java-chassis-samples/blob/master/porter/gateway-service/src/main/java/com/huawei/cse/porter/gateway/UiDispatcher.java I suggest add a configuration, e.g. servicecomb.flowcontrol.transport.enable to make flow controll logic executed before handler, and is a standalone logic, and you can resuse the code of handler. And the execution will not influence any handler logic(that's executed one by one, and if you configured flowcontrol handler, it will also be executed, that's twice execution). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16704152#comment-16704152
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r237726639
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
Review comment:
Will here cause all handlers executed?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[
https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16704153#comment-16704153
]
ASF GitHub Bot commented on SCB-1056:
-
liubao68 commented on a change in pull request #1017: [SCB-1056] put provider
flow control logic in front
URL:
https://github.com/apache/servicecomb-java-chassis/pull/1017#discussion_r237726716
##
File path:
common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
##
@@ -150,6 +167,39 @@ protected void scheduleInvocation() {
});
}
+ private Holder checkQpsFlowControl(OperationMeta operationMeta) {
+Holder qpsFlowControlReject = new Holder<>(false);
+Handler providerQpsFlowControlHandler =
findQpsFlowControlHandler(operationMeta);
+if (null != providerQpsFlowControlHandler) {
+ try {
+providerQpsFlowControlHandler.handle(invocation, response -> {
+ qpsFlowControlReject.value = true;
+ produceProcessor = ProduceProcessorManager.JSON_PROCESSOR;
+ sendResponse(response);
+});
+ } catch (Exception e) {
+LOGGER.error("failed to execute ProviderQpsFlowControlHandler", e);
+qpsFlowControlReject.value = true;
+sendFailResponse(e);
+ }
+}
+return qpsFlowControlReject;
Review comment:
The async result may not be present
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Put provider QPS flow control in front
> --
>
> Key: SCB-1056
> URL: https://issues.apache.org/jira/browse/SCB-1056
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: YaoHaishi
>Assignee: YaoHaishi
>Priority: Major
>
> Currently provider QPS flow control is in ProviderQpsFlowControlHandler which
> works in provider handler chain. As a result, the flow control logic takes
> effect too late and much CPU resource is wasted on processing those requests
> that should be rejected earlier.
> Put the provider QPS flow control logic in front can save the resource.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16703084#comment-16703084 ] ASF GitHub Bot commented on SCB-1056: - coveralls commented on issue #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017#issuecomment-442809880 [](https://coveralls.io/builds/20357299) Coverage decreased (-0.001%) to 86.773% when pulling **cae34b6c54e9522ba8dec767af1dc54967c75ccb on yhs0092:enhance_QPS_limit** into **47d4773f3db7444e07fa2bb5bf87ac0a0387855f on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-1056) Put provider QPS flow control in front
[ https://issues.apache.org/jira/browse/SCB-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16703066#comment-16703066 ] ASF GitHub Bot commented on SCB-1056: - yhs0092 opened a new pull request #1017: [SCB-1056] put provider flow control logic in front URL: https://github.com/apache/servicecomb-java-chassis/pull/1017 Follow this checklist to help us incorporate your contribution quickly and easily: - [x] 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. - [x] Each commit in the pull request should have a meaningful subject line and body. - [x] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- See details in [SCB-1056](https://issues.apache.org/jira/browse/SCB-1056) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Put provider QPS flow control in front > -- > > Key: SCB-1056 > URL: https://issues.apache.org/jira/browse/SCB-1056 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: YaoHaishi >Assignee: YaoHaishi >Priority: Major > > Currently provider QPS flow control is in ProviderQpsFlowControlHandler which > works in provider handler chain. As a result, the flow control logic takes > effect too late and much CPU resource is wasted on processing those requests > that should be rejected earlier. > Put the provider QPS flow control logic in front can save the resource. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
