This is an automated email from the ASF dual-hosted git repository. wujimin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit c018bed9051cd8d7c27c07ec1824bc554d409eb0 Author: liubao68 <bi...@qq.com> AuthorDate: Sat Mar 20 15:07:28 2021 +0800 [SCB-2216]breakpoints testing for fast timeout of execution --- .../common/rest/AbstractRestInvocation.java | 20 ++- .../java/org/apache/servicecomb/core/Const.java | 6 + .../org/apache/servicecomb/core/Invocation.java | 45 ++++-- .../core/definition/OperationConfig.java | 21 +++ .../InvocationBusinessFinishEvent.java} | 14 +- .../InvocationHandlersStartEvent.java} | 16 +-- .../InvocationStartSendRequestEvent.java} | 16 +-- .../InvocationTimeoutCheckEvent.java} | 16 +-- .../servicecomb/core/exception/ExceptionCodes.java | 1 + .../core/filter/impl/ScheduleFilter.java | 2 +- .../invocation/InvocationTimeoutBootListener.java | 159 +++++++++++++++++++++ .../core/provider/consumer/InvokerUtils.java | 7 +- .../core/definition/OperationConfigTest.java | 9 ++ .../src/main/resources/microservice.yaml | 3 + .../src/main/resources/microservice.yaml | 5 +- .../servicecomb/demo/CommonSchemaInterface.java | 36 +++++ .../java/org/apache/servicecomb/demo/TestMgr.java | 4 + .../client/TestSpringMVCCommonSchemaInterface.java | 76 ++++++++++ .../demo/springmvc/client/TestUploadSchema.java | 2 + .../src/main/resources/microservice.yaml | 1 - .../springmvc/server/ProducerTestsAfterBootup.java | 4 +- .../server/SpringMVCCommonSchemaInterface.java | 60 ++++++++ .../demo/springmvc/server/UploadSchema.java | 4 + .../src/main/resources/microservice.yaml | 6 + .../foundation/common/event/SimpleSubscriber.java | 2 +- .../bizkeeper/FallbackPolicyManager.java | 6 +- .../bizkeeper/TestFallbackPolicyManager.java | 8 +- .../transport/highway/HighwayClient.java | 2 +- .../transport/highway/HighwayCodec.java | 1 - .../transport/highway/HighwayServerConnection.java | 2 + .../transport/highway/HighwayServerInvoke.java | 7 +- .../transport/highway/TestHighwayClient.java | 4 - .../transport/highway/TestHighwayCodec.java | 6 +- .../rest/client/RestClientSenderFilter.java | 2 + .../rest/client/http/RestClientInvocation.java | 6 +- .../rest/client/http/TestRestClientInvocation.java | 3 - 36 files changed, 493 insertions(+), 89 deletions(-) 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 2304022..5041d6c 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 @@ -126,10 +126,6 @@ public abstract class AbstractRestInvocation { return; } - invocation.onStart(requestEx, start); - invocation.getInvocationStageTrace().startSchedule(); - OperationMeta operationMeta = restOperationMeta.getOperationMeta(); - try { this.setContext(); } catch (Exception e) { @@ -137,6 +133,10 @@ public abstract class AbstractRestInvocation { sendFailResponse(e); return; } + + invocation.onStart(requestEx, start); + invocation.getInvocationStageTrace().startSchedule(); + OperationMeta operationMeta = restOperationMeta.getOperationMeta(); Holder<Boolean> qpsFlowControlReject = checkQpsFlowControl(operationMeta); if (qpsFlowControlReject.value) { @@ -160,8 +160,11 @@ public abstract class AbstractRestInvocation { } runOnExecutor(); + } catch (InvocationException e) { + LOGGER.error("Invocation failed, cause={}", e.getMessage()); + sendFailResponse(e); } catch (Throwable e) { - LOGGER.error("rest server onRequest error", e); + LOGGER.error("Processing rest server request error", e); sendFailResponse(e); } } @@ -217,8 +220,11 @@ public abstract class AbstractRestInvocation { } doInvoke(); + } catch (InvocationException e) { + LOGGER.error("Invocation failed, cause={}", e.getMessage()); + sendFailResponse(e); } catch (Throwable e) { - LOGGER.error("unknown rest exception.", e); + LOGGER.error("Processing rest server request error", e); sendFailResponse(e); } } @@ -241,7 +247,7 @@ public abstract class AbstractRestInvocation { } protected void doInvoke() throws Throwable { - invocation.getInvocationStageTrace().startHandlersRequest(); + invocation.onStartHandlersRequest(); invocation.next(resp -> sendResponseQuietly(resp)); } diff --git a/core/src/main/java/org/apache/servicecomb/core/Const.java b/core/src/main/java/org/apache/servicecomb/core/Const.java index fc90ac0..e4940aa 100644 --- a/core/src/main/java/org/apache/servicecomb/core/Const.java +++ b/core/src/main/java/org/apache/servicecomb/core/Const.java @@ -25,6 +25,12 @@ public final class Const { public static final String CSE_CONTEXT = "x-cse-context"; + public static final String CONTEXT_TIME_ELAPSED = "x-scb-time"; + + public static final String CONTEXT_TIMED_OUT = "x-scb-timed-out"; + + public static final String CONTEXT_TIME_CURRENT = "x-scb-time-current"; + public static final String RESTFUL = "rest"; public static final String HIGHWAY = "highway"; diff --git a/core/src/main/java/org/apache/servicecomb/core/Invocation.java b/core/src/main/java/org/apache/servicecomb/core/Invocation.java index 88f42bc..30fe533 100644 --- a/core/src/main/java/org/apache/servicecomb/core/Invocation.java +++ b/core/src/main/java/org/apache/servicecomb/core/Invocation.java @@ -32,13 +32,17 @@ import org.apache.servicecomb.core.definition.InvocationRuntimeType; import org.apache.servicecomb.core.definition.MicroserviceMeta; import org.apache.servicecomb.core.definition.OperationMeta; import org.apache.servicecomb.core.definition.SchemaMeta; +import org.apache.servicecomb.core.event.InvocationBusinessFinishEvent; import org.apache.servicecomb.core.event.InvocationBusinessMethodFinishEvent; import org.apache.servicecomb.core.event.InvocationBusinessMethodStartEvent; import org.apache.servicecomb.core.event.InvocationEncodeResponseStartEvent; import org.apache.servicecomb.core.event.InvocationFinishEvent; +import org.apache.servicecomb.core.event.InvocationHandlersStartEvent; import org.apache.servicecomb.core.event.InvocationRunInExecutorFinishEvent; import org.apache.servicecomb.core.event.InvocationRunInExecutorStartEvent; import org.apache.servicecomb.core.event.InvocationStartEvent; +import org.apache.servicecomb.core.event.InvocationStartSendRequestEvent; +import org.apache.servicecomb.core.event.InvocationTimeoutCheckEvent; import org.apache.servicecomb.core.invocation.InvocationStageTrace; import org.apache.servicecomb.core.provider.consumer.InvokerUtils; import org.apache.servicecomb.core.provider.consumer.ReferenceConfig; @@ -52,6 +56,7 @@ import org.apache.servicecomb.swagger.invocation.AsyncResponse; import org.apache.servicecomb.swagger.invocation.InvocationType; import org.apache.servicecomb.swagger.invocation.Response; import org.apache.servicecomb.swagger.invocation.SwaggerInvocation; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import com.fasterxml.jackson.databind.JavaType; @@ -136,16 +141,6 @@ public class Invocation extends SwaggerInvocation { return getContext(traceIdName); } - @Deprecated - public long getStartTime() { - return invocationStageTrace.getStart(); - } - - @Deprecated - public long getStartExecutionTime() { - return invocationStageTrace.getStartExecution(); - } - public Invocation() { // An empty invocation, used to mock or some other scenario do not need operation information. traceIdLogger = new TraceIdLogger(this); @@ -276,10 +271,6 @@ public class Invocation extends SwaggerInvocation { return producerArguments = args; } - public void clearProducerArguments() { - producerArguments = null; - } - public Endpoint getEndpoint() { return endpoint; } @@ -411,6 +402,16 @@ public class Invocation extends SwaggerInvocation { EventManager.post(new InvocationRunInExecutorFinishEvent(this)); } + public void onStartHandlersRequest() { + invocationStageTrace.startHandlersRequest(); + EventManager.post(new InvocationHandlersStartEvent(this)); + } + + public void onStartSendRequest() { + invocationStageTrace.startSend(); + EventManager.post(new InvocationStartSendRequestEvent(this)); + } + @Override public void onBusinessMethodStart() { invocationStageTrace.startBusinessMethod(); @@ -429,6 +430,7 @@ public class Invocation extends SwaggerInvocation { @Override public void onBusinessFinish() { invocationStageTrace.finishBusiness(); + EventManager.post(new InvocationBusinessFinishEvent(this)); } public void onFinish(Response response) { @@ -482,4 +484,19 @@ public class Invocation extends SwaggerInvocation { return future; } + + /** + * Check if invocation is timeout. + * + * NOTICE: this method only trigger event to ask the target checker to do the real check. So this method + * will only take effect when timeout checker is enabled. + * + * e.g. InvocationTimeoutBootListener.ENABLE_TIMEOUT_CHECK is enabled. + * + * @throws InvocationException if timeout, throw an exception. Will not throw exception twice if this method called + * after timeout. + */ + public void ensureInvocationNotTimeout() throws InvocationException { + EventManager.post(new InvocationTimeoutCheckEvent(this)); + } } diff --git a/core/src/main/java/org/apache/servicecomb/core/definition/OperationConfig.java b/core/src/main/java/org/apache/servicecomb/core/definition/OperationConfig.java index ccbbb68..c210efd 100644 --- a/core/src/main/java/org/apache/servicecomb/core/definition/OperationConfig.java +++ b/core/src/main/java/org/apache/servicecomb/core/definition/OperationConfig.java @@ -65,6 +65,14 @@ public class OperationConfig { private long msRequestTimeout; /** + * Invocation timeout. + */ + @InjectProperty(keys = {"invocation.${op-any-priority}.timeout", "invocation.timeout"}, defaultValue = "-1") + private long msInvocationTimeout; + + private long nanoInvocationTimeout; + + /** * whether to remove certain headers from the 3rd party invocations */ @InjectProperty(keys = {"request.clientRequestHeaderFilterEnabled${consumer-op-priority}"}, defaultValue = "true") @@ -178,6 +186,19 @@ public class OperationConfig { return nanoRestRequestWaitInPoolTimeout; } + public long getMsInvocationTimeout() { + return msInvocationTimeout; + } + + public void setMsInvocationTimeout(long msInvocationTimeout) { + this.msInvocationTimeout = msInvocationTimeout; + this.nanoInvocationTimeout = TimeUnit.MILLISECONDS.toNanos(msInvocationTimeout); + } + + public long getNanoInvocationTimeout() { + return this.nanoInvocationTimeout; + } + public boolean isClientRequestHeaderFilterEnabled() { return clientRequestHeaderFilterEnabled; } diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/event/InvocationBusinessFinishEvent.java similarity index 72% copy from core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java copy to core/src/main/java/org/apache/servicecomb/core/event/InvocationBusinessFinishEvent.java index f28b4e6..2c3fe13 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java +++ b/core/src/main/java/org/apache/servicecomb/core/event/InvocationBusinessFinishEvent.java @@ -14,13 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.servicecomb.core.exception; -public interface ExceptionCodes { - String GENERIC_CLIENT = "SCB.00000000"; - String LB_ADDRESS_NOT_FOUND = "SCB.00000001"; - String NOT_DEFINED_ANY_SCHEMA = "SCB.00000002"; - String DEFAULT_VALIDATE = "SCB.00000003"; +package org.apache.servicecomb.core.event; - String GENERIC_SERVER = "SCB.50000000"; +import org.apache.servicecomb.core.Invocation; + +public class InvocationBusinessFinishEvent extends InvocationBaseEvent { + public InvocationBusinessFinishEvent(Invocation invocation) { + super(invocation); + } } diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/event/InvocationHandlersStartEvent.java similarity index 72% copy from core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java copy to core/src/main/java/org/apache/servicecomb/core/event/InvocationHandlersStartEvent.java index f28b4e6..a064b16 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java +++ b/core/src/main/java/org/apache/servicecomb/core/event/InvocationHandlersStartEvent.java @@ -14,13 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.servicecomb.core.exception; -public interface ExceptionCodes { - String GENERIC_CLIENT = "SCB.00000000"; - String LB_ADDRESS_NOT_FOUND = "SCB.00000001"; - String NOT_DEFINED_ANY_SCHEMA = "SCB.00000002"; - String DEFAULT_VALIDATE = "SCB.00000003"; +package org.apache.servicecomb.core.event; - String GENERIC_SERVER = "SCB.50000000"; -} +import org.apache.servicecomb.core.Invocation; + +public class InvocationHandlersStartEvent extends InvocationBaseEvent { + public InvocationHandlersStartEvent(Invocation invocation) { + super(invocation); + } +} \ No newline at end of file diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/event/InvocationStartSendRequestEvent.java similarity index 72% copy from core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java copy to core/src/main/java/org/apache/servicecomb/core/event/InvocationStartSendRequestEvent.java index f28b4e6..a5bd8c8 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java +++ b/core/src/main/java/org/apache/servicecomb/core/event/InvocationStartSendRequestEvent.java @@ -14,13 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.servicecomb.core.exception; -public interface ExceptionCodes { - String GENERIC_CLIENT = "SCB.00000000"; - String LB_ADDRESS_NOT_FOUND = "SCB.00000001"; - String NOT_DEFINED_ANY_SCHEMA = "SCB.00000002"; - String DEFAULT_VALIDATE = "SCB.00000003"; +package org.apache.servicecomb.core.event; - String GENERIC_SERVER = "SCB.50000000"; -} +import org.apache.servicecomb.core.Invocation; + +public class InvocationStartSendRequestEvent extends InvocationBaseEvent { + public InvocationStartSendRequestEvent(Invocation invocation) { + super(invocation); + } +} \ No newline at end of file diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/event/InvocationTimeoutCheckEvent.java similarity index 72% copy from core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java copy to core/src/main/java/org/apache/servicecomb/core/event/InvocationTimeoutCheckEvent.java index f28b4e6..168081e 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java +++ b/core/src/main/java/org/apache/servicecomb/core/event/InvocationTimeoutCheckEvent.java @@ -14,13 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.servicecomb.core.exception; -public interface ExceptionCodes { - String GENERIC_CLIENT = "SCB.00000000"; - String LB_ADDRESS_NOT_FOUND = "SCB.00000001"; - String NOT_DEFINED_ANY_SCHEMA = "SCB.00000002"; - String DEFAULT_VALIDATE = "SCB.00000003"; +package org.apache.servicecomb.core.event; - String GENERIC_SERVER = "SCB.50000000"; -} +import org.apache.servicecomb.core.Invocation; + +public class InvocationTimeoutCheckEvent extends InvocationBaseEvent { + public InvocationTimeoutCheckEvent(Invocation invocation) { + super(invocation); + } +} \ No newline at end of file diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java b/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java index f28b4e6..4c2c508 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java +++ b/core/src/main/java/org/apache/servicecomb/core/exception/ExceptionCodes.java @@ -21,6 +21,7 @@ public interface ExceptionCodes { String LB_ADDRESS_NOT_FOUND = "SCB.00000001"; String NOT_DEFINED_ANY_SCHEMA = "SCB.00000002"; String DEFAULT_VALIDATE = "SCB.00000003"; + String INVOCATION_TIMEOUT = "SCB.00000004"; String GENERIC_SERVER = "SCB.50000000"; } diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java index a07ff75..e144d81 100644 --- a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java +++ b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ScheduleFilter.java @@ -53,7 +53,7 @@ public class ScheduleFilter implements ProducerFilter { try { InvocationStageTrace trace = invocation.getInvocationStageTrace(); trace.startServerFiltersRequest(); - trace.startHandlersRequest(); + invocation.onStartHandlersRequest(); checkInQueueTimeout(invocation); diff --git a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java new file mode 100644 index 0000000..3aa3327 --- /dev/null +++ b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java @@ -0,0 +1,159 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.servicecomb.core.invocation; + +import static javax.ws.rs.core.Response.Status.REQUEST_TIMEOUT; + +import org.apache.commons.lang3.StringUtils; +import org.apache.servicecomb.core.BootListener; +import org.apache.servicecomb.core.Const; +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.core.event.InvocationBusinessFinishEvent; +import org.apache.servicecomb.core.event.InvocationBusinessMethodStartEvent; +import org.apache.servicecomb.core.event.InvocationFinishEvent; +import org.apache.servicecomb.core.event.InvocationHandlersStartEvent; +import org.apache.servicecomb.core.event.InvocationRunInExecutorStartEvent; +import org.apache.servicecomb.core.event.InvocationStartEvent; +import org.apache.servicecomb.core.event.InvocationStartSendRequestEvent; +import org.apache.servicecomb.core.event.InvocationTimeoutCheckEvent; +import org.apache.servicecomb.core.exception.ExceptionCodes; +import org.apache.servicecomb.foundation.common.event.EnableExceptionPropagation; +import org.apache.servicecomb.foundation.common.event.EventManager; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +import com.google.common.eventbus.Subscribe; +import com.netflix.config.DynamicPropertyFactory; + +@Component +public class InvocationTimeoutBootListener implements BootListener { + private static final Logger LOGGER = LoggerFactory.getLogger(InvocationTimeoutBootListener.class); + + public static final String ENABLE_TIMEOUT_CHECK = "servicecomb.invocation.enableTimeoutCheck"; + + public static boolean timeoutCheckEnabled() { + return DynamicPropertyFactory.getInstance().getBooleanProperty + (ENABLE_TIMEOUT_CHECK, true).get(); + } + + @Override + public void onAfterRegistry(BootEvent event) { + if (timeoutCheckEnabled()) { + EventManager.getEventBus().register(this); + } + } + + @Subscribe + public void onInvocationStartEvent(InvocationStartEvent event) { + Invocation invocation = event.getInvocation(); + + // not initialized + // 1. when first time received request + // 2. when first time send request not a user thread + // initialized + // 1. send request in the progress of processing request + if (invocation.getLocalContext(Const.CONTEXT_TIME_CURRENT) == null) { + invocation.addLocalContext(Const.CONTEXT_TIME_CURRENT, invocation.getInvocationStageTrace().getStart()); + } + + if (invocation.getLocalContext(Const.CONTEXT_TIME_ELAPSED) == null) { + String elapsed = invocation.getContext(Const.CONTEXT_TIME_ELAPSED); + if (StringUtils.isEmpty(elapsed)) { + invocation.addLocalContext(Const.CONTEXT_TIME_ELAPSED, 0L); + return; + } + + try { + invocation.addLocalContext(Const.CONTEXT_TIME_ELAPSED, Long.parseLong(elapsed)); + } catch (NumberFormatException e) { + LOGGER.error("Not expected number format exception, attacker?"); + invocation.addLocalContext(Const.CONTEXT_TIME_ELAPSED, 0L); + } + } + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationTimeoutCheckEvent(InvocationTimeoutCheckEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } + + /** + * check if invocation is timeout. + * + * @throws InvocationException if timeout, throw an exception. Will not throw exception twice if this method called + * after timeout. + */ + private void ensureInvocationNotTimeout(Invocation invocation) { + if (invocation.getOperationMeta().getConfig().getNanoInvocationTimeout() > 0 && calculateElapsedTime(invocation) > + invocation.getOperationMeta().getConfig().getNanoInvocationTimeout()) { + if (invocation.getLocalContext(Const.CONTEXT_TIMED_OUT) != null) { + // already timed out, do not throw exception again + return; + } + invocation.addLocalContext(Const.CONTEXT_TIMED_OUT, true); + throw new InvocationException(REQUEST_TIMEOUT, + ExceptionCodes.INVOCATION_TIMEOUT, "Invocation Timeout."); + } + } + + private long calculateElapsedTime(Invocation invocation) { + return System.nanoTime() - (long) invocation.getLocalContext(Const.CONTEXT_TIME_CURRENT) + + (long) invocation.getLocalContext(Const.CONTEXT_TIME_ELAPSED); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationRunInExecutorStartEvent(InvocationRunInExecutorStartEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationHandlersStartEvent(InvocationHandlersStartEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationBusinessMethodStartEvent(InvocationBusinessMethodStartEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationBusinessFinishEvent(InvocationBusinessFinishEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationStartSendRequestEvent(InvocationStartSendRequestEvent event) { + Invocation invocation = event.getInvocation(); + ensureInvocationNotTimeout(invocation); + invocation.addContext(Const.CONTEXT_TIME_ELAPSED, Long.toString(calculateElapsedTime(invocation))); + } + + @Subscribe + @EnableExceptionPropagation + public void onInvocationFinishEvent(InvocationFinishEvent event) { + ensureInvocationNotTimeout(event.getInvocation()); + } +} diff --git a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java index c14cc24..09b4337 100644 --- a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java +++ b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java @@ -154,7 +154,7 @@ public final class InvokerUtils { SyncResponseExecutor respExecutor = new SyncResponseExecutor(); invocation.setResponseExecutor(respExecutor); - invocation.getInvocationStageTrace().startHandlersRequest(); + invocation.onStartHandlersRequest(); invocation.next(respExecutor::setResponse); Response response = respExecutor.waitResponse(); @@ -187,7 +187,7 @@ public final class InvokerUtils { ReactiveResponseExecutor respExecutor = new ReactiveResponseExecutor(); invocation.setResponseExecutor(respExecutor); - invocation.getInvocationStageTrace().startHandlersRequest(); + invocation.onStartHandlersRequest(); invocation.next(ar -> { ContextUtils.setInvocationContext(invocation.getParentContext()); try { @@ -225,8 +225,7 @@ public final class InvokerUtils { */ public static CompletableFuture<Response> invoke(Invocation invocation) { invocation.onStart(null, System.nanoTime()); - invocation.getInvocationStageTrace().startHandlersRequest(); - + invocation.onStartHandlersRequest(); return invocation.getMicroserviceMeta().getFilterChain() .onFilter(invocation) .exceptionally(throwable -> convertException(invocation, throwable)) diff --git a/core/src/test/java/org/apache/servicecomb/core/definition/OperationConfigTest.java b/core/src/test/java/org/apache/servicecomb/core/definition/OperationConfigTest.java index 1745dfc..dc8076b 100644 --- a/core/src/test/java/org/apache/servicecomb/core/definition/OperationConfigTest.java +++ b/core/src/test/java/org/apache/servicecomb/core/definition/OperationConfigTest.java @@ -58,5 +58,14 @@ class OperationConfigTest { long nano = TimeUnit.MILLISECONDS.toNanos(2); assertThat(config.getNanoRequestWaitInPoolTimeout("abc")).isEqualTo(nano); } + + @Test + void should_get_invocation_timeout_value() { + config.setMsInvocationTimeout(1); + + long nano = TimeUnit.MILLISECONDS.toNanos(1); + assertThat(config.getNanoInvocationTimeout()).isEqualTo(nano); + assertThat(config.getMsInvocationTimeout()).isEqualTo(1); + } } } \ No newline at end of file diff --git a/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml b/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml index 89a8e48..fd46e82 100644 --- a/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml +++ b/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml @@ -43,6 +43,9 @@ servicecomb: add: timeout: 1000 + invocation: + enableTimeoutCheck: false + # test configurations. you can choose any implementation. default using local. # using nacos configuration diff --git a/demo/demo-jaxrs/jaxrs-server/src/main/resources/microservice.yaml b/demo/demo-jaxrs/jaxrs-server/src/main/resources/microservice.yaml index d6da5b2..2eda948 100644 --- a/demo/demo-jaxrs/jaxrs-server/src/main/resources/microservice.yaml +++ b/demo/demo-jaxrs/jaxrs-server/src/main/resources/microservice.yaml @@ -36,4 +36,7 @@ servicecomb: codec: printErrorMessage: true - executors.Provider.ReactiveSchema: servicecomb.executor.reactive \ No newline at end of file + executors.Provider.ReactiveSchema: servicecomb.executor.reactive + + invocation: + enableTimeoutCheck: false \ No newline at end of file diff --git a/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/CommonSchemaInterface.java b/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/CommonSchemaInterface.java new file mode 100644 index 0000000..0b50fa6 --- /dev/null +++ b/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/CommonSchemaInterface.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.servicecomb.demo; + +import org.apache.servicecomb.swagger.invocation.context.InvocationContext; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; + +import io.swagger.annotations.ApiOperation; + +@RequestMapping("/CommonSchemaInterface") +public interface CommonSchemaInterface { + @GetMapping(path = "testInvocationTimeout") + String testInvocationTimeout(@RequestParam("timeout") long timeout, @RequestParam("name") String name); + + @GetMapping(path = "testInvocationTimeoutWithInvocation") + @ApiOperation(value = "testInvocationTimeoutWithInvocation", nickname = "testInvocationTimeoutWithInvocation") + String testInvocationTimeout(InvocationContext context, @RequestParam("timeout") long timeout, + @RequestParam("name") String name); +} diff --git a/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/TestMgr.java b/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/TestMgr.java index ede3da4..eb80dc6 100644 --- a/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/TestMgr.java +++ b/demo/demo-schema/src/main/java/org/apache/servicecomb/demo/TestMgr.java @@ -73,6 +73,10 @@ public class TestMgr { } } + public static void fail(String desc) { + failed(desc, new Exception(desc)); + } + public static void failed(String desc, Throwable e) { checkes.incrementAndGet(); diff --git a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestSpringMVCCommonSchemaInterface.java b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestSpringMVCCommonSchemaInterface.java new file mode 100644 index 0000000..e45da0d --- /dev/null +++ b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestSpringMVCCommonSchemaInterface.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.servicecomb.demo.springmvc.client; + +import java.util.concurrent.TimeUnit; + +import org.apache.servicecomb.core.Const; +import org.apache.servicecomb.demo.CategorizedTestCase; +import org.apache.servicecomb.demo.CommonSchemaInterface; +import org.apache.servicecomb.demo.TestMgr; +import org.apache.servicecomb.provider.pojo.RpcReference; +import org.apache.servicecomb.swagger.invocation.context.InvocationContext; +import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; +import org.springframework.stereotype.Component; + +@Component +public class TestSpringMVCCommonSchemaInterface implements CategorizedTestCase { + @RpcReference(schemaId = "SpringMVCCommonSchemaInterface", microserviceName = "springmvc") + private CommonSchemaInterface client; + + public void testAllTransport() throws Exception { + testInvocationTimeoutInServer(); + testInvocationTimeoutInServerUserCheck(); + testInvocationAlreadyTimeoutInClient(); + } + + private void testInvocationTimeoutInServerUserCheck() { + try { + InvocationContext context = new InvocationContext(); + client.testInvocationTimeout(context, 1001, "customized"); + TestMgr.fail("should timeout"); + } catch (InvocationException e) { + TestMgr.check(408, e.getStatusCode()); + TestMgr.check("Invocation Timeout.", ((CommonExceptionData) e.getErrorData()).getMessage()); + } + } + + private void testInvocationAlreadyTimeoutInClient() { + try { + InvocationContext context = new InvocationContext(); + context.addLocalContext(Const.CONTEXT_TIME_CURRENT, System.nanoTime()); + context.addLocalContext(Const.CONTEXT_TIME_ELAPSED, TimeUnit.SECONDS.toNanos(1)); + client.testInvocationTimeout(context, 1, "hello"); + TestMgr.fail("should timeout"); + } catch (InvocationException e) { + TestMgr.check(408, e.getStatusCode()); + TestMgr.check("Invocation Timeout.", ((CommonExceptionData) e.getErrorData()).getMessage()); + } + } + + private void testInvocationTimeoutInServer() { + try { + client.testInvocationTimeout(1001, "hello"); + TestMgr.fail("should timeout"); + } catch (InvocationException e) { + TestMgr.check(408, e.getStatusCode()); + TestMgr.check("Invocation Timeout.", ((CommonExceptionData) e.getErrorData()).getMessage()); + } + } +} diff --git a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestUploadSchema.java b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestUploadSchema.java index cebd61c..c43f8e2 100644 --- a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestUploadSchema.java +++ b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestUploadSchema.java @@ -78,5 +78,7 @@ public class TestUploadSchema implements CategorizedTestCase { String result = template.postForObject("servicecomb://springmvc/upload/fileUpload", entity, String.class); TestMgr.check(result, "success"); + + files.forEach(file -> file.delete()); } } diff --git a/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml b/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml index cc63f46..efd5bdf 100644 --- a/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml +++ b/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml @@ -34,7 +34,6 @@ servicecomb: pull: interval: 90 watch: true - autodiscovery: true # can download config center from https://cse-bucket.obs.myhwclouds.com/LocalCSE/Local-CSE-1.0.0.zip to test dynamic config config: client: diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ProducerTestsAfterBootup.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ProducerTestsAfterBootup.java index 5ce24df..e36dcd7 100644 --- a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ProducerTestsAfterBootup.java +++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ProducerTestsAfterBootup.java @@ -91,9 +91,9 @@ public class ProducerTestsAfterBootup implements BootListener { public void testRegisteredBasePath() { if (DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.test.vert.transport", true).get()) { - TestMgr.check(18, RegistrationManager.INSTANCE.getMicroservice().getPaths().size()); + TestMgr.check(19, RegistrationManager.INSTANCE.getMicroservice().getPaths().size()); } else { - TestMgr.check(19, RegistrationManager.INSTANCE.getMicroservice().getPaths().size()); + TestMgr.check(20, RegistrationManager.INSTANCE.getMicroservice().getPaths().size()); } } diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMVCCommonSchemaInterface.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMVCCommonSchemaInterface.java new file mode 100644 index 0000000..4fa5b73 --- /dev/null +++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/SpringMVCCommonSchemaInterface.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.servicecomb.demo.springmvc.server; + +import javax.ws.rs.core.Response.Status; + +import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.demo.CommonSchemaInterface; +import org.apache.servicecomb.provider.rest.common.RestSchema; +import org.apache.servicecomb.swagger.invocation.context.InvocationContext; +import org.apache.servicecomb.swagger.invocation.exception.InvocationException; + +@RestSchema(schemaId = "SpringMVCCommonSchemaInterface", schemaInterface = CommonSchemaInterface.class) +public class SpringMVCCommonSchemaInterface implements CommonSchemaInterface { + @Override + public String testInvocationTimeout(long timeout, String name) { + try { + Thread.sleep(timeout); + } catch (InterruptedException e) { + + } + + return name; + } + + @Override + public String testInvocationTimeout(InvocationContext context, long timeout, + String name) { + + if ("customized".equals(name)) { + try { + Thread.sleep(timeout); + } catch (InterruptedException e) { + + } + + Invocation invocation = (Invocation) context; + invocation.ensureInvocationNotTimeout(); + + throw new InvocationException(Status.BAD_REQUEST, "not expected result"); + } + + return testInvocationTimeout(timeout, name); + } +} diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/UploadSchema.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/UploadSchema.java index 24ec3f2..110ccd8 100644 --- a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/UploadSchema.java +++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/UploadSchema.java @@ -19,6 +19,7 @@ package org.apache.servicecomb.demo.springmvc.server; import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -39,12 +40,15 @@ public class UploadSchema { public String fileUpload(@RequestPart(name = "files") List<MultipartFile> files) { try { String fileName = UUID.randomUUID().toString(); + List<File> savedFiles = new ArrayList<>(); int index = 0; for (MultipartFile file : files) { File tempFile = new File("random-server-" + fileName + index); + savedFiles.add(tempFile); file.transferTo(tempFile); index++; } + savedFiles.forEach(file -> file.delete()); return "success"; } catch (IOException e) { return "failed"; diff --git a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml index 4ebec70..ef9141d 100644 --- a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml +++ b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml @@ -60,6 +60,12 @@ servicecomb: compression: true highway: address: 0.0.0.0:7070?sslEnabled=true + invocation: + SpringMVCCommonSchemaInterface: + testInvocationTimeout: + timeout: 1000 + testInvocationTimeoutWithInvocation: + timeout: 1000 handler: chain: Provider: diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java index e209f44..561974c 100644 --- a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java +++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/event/SimpleSubscriber.java @@ -107,9 +107,9 @@ public class SimpleSubscriber { dispatcher.accept(event); } catch (Throwable e) { if (enableExceptionPropagation) { - LOGGER.error("event process should not throw error. ", e); throw e; } + LOGGER.error("Event process should not throw exception when @EnableExceptionPropagation not set. ", e); } } diff --git a/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/FallbackPolicyManager.java b/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/FallbackPolicyManager.java index 88f502a..03da31d 100644 --- a/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/FallbackPolicyManager.java +++ b/handlers/handler-bizkeeper/src/main/java/org/apache/servicecomb/bizkeeper/FallbackPolicyManager.java @@ -41,11 +41,7 @@ public class FallbackPolicyManager { if (policy != null) { return policy.getFallbackResponse(invocation, error); } else { - return Response.failResp(invocation.getInvocationType(), - BizkeeperExceptionUtils - .createBizkeeperException(BizkeeperExceptionUtils.SERVICECOMB_BIZKEEPER_FALLBACK, - error, - invocation.getOperationMeta().getMicroserviceQualifiedName())); + return Response.failResp(invocation.getInvocationType(), error); } } diff --git a/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/TestFallbackPolicyManager.java b/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/TestFallbackPolicyManager.java index 4860146..a9c0f70 100644 --- a/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/TestFallbackPolicyManager.java +++ b/handlers/handler-bizkeeper/src/test/java/org/apache/servicecomb/bizkeeper/TestFallbackPolicyManager.java @@ -16,6 +16,8 @@ */ package org.apache.servicecomb.bizkeeper; +import javax.ws.rs.core.Response.Status; + import org.apache.servicecomb.core.Invocation; import org.apache.servicecomb.core.definition.OperationMeta; import org.apache.servicecomb.core.exception.CseException; @@ -134,8 +136,8 @@ public class TestFallbackPolicyManager { result = "unknown"; } }; - Assert.assertEquals(CseException.class, - ((Exception) FallbackPolicyManager.getFallbackResponse("Consumer", null, invocation).getResult()).getCause() - .getClass()); + Assert.assertEquals(InvocationException.class, + ((Exception) FallbackPolicyManager.getFallbackResponse("Consumer", new InvocationException( + Status.TOO_MANY_REQUESTS, ""), invocation).getResult()).getClass()); } } diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java index 630a095..cbf233a 100644 --- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java +++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java @@ -87,7 +87,7 @@ public class HighwayClient { public void send(Invocation invocation, AsyncResponse asyncResp) throws Exception { invocation.getInvocationStageTrace().startClientFiltersRequest(); - invocation.getInvocationStageTrace().startSend(); + invocation.onStartSendRequest(); HighwayClientConnection tcpClient = findClientPool(invocation); diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java index ac2bb96..937a62b 100644 --- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java +++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java @@ -81,7 +81,6 @@ public final class HighwayCodec { Map<String, Object> swaggerArguments = requestDeserializer.deserialize(bodyBuffer.getBytes()); addPrimitiveTypeDefaultValues(invocation, swaggerArguments); invocation.setSwaggerArguments(swaggerArguments); - invocation.mergeContext(header.getContext()); } public static RequestHeader readRequestHeader(Buffer headerBuffer) throws Exception { diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerConnection.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerConnection.java index bb50fbc..a646f7a 100644 --- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerConnection.java +++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerConnection.java @@ -156,6 +156,8 @@ public class HighwayServerConnection extends TcpServerConnection implements TcpB .setOperationProtobuf(ProtobufManager.getOrCreateOperation(invocation)); invocation.setTransportContext(transportContext); + invocation.mergeContext(header.getContext()); + return CompletableFuture.completedFuture(invocation); } } 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 2ffe63b..c88e90d 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 @@ -128,7 +128,7 @@ public class HighwayServerInvoke { HighwayCodec.decodeRequest(invocation, header, operationProtobuf, bodyBuffer); invocation.getHandlerContext().put(Const.REMOTE_ADDRESS, this.connection.getNetSocket().remoteAddress()); - invocation.getInvocationStageTrace().startHandlersRequest(); + invocation.onStartHandlersRequest(); invocation.next(response -> sendResponse(invocation.getContext(), response)); } @@ -180,14 +180,11 @@ public class HighwayServerInvoke { .setBodyBuffer(bodyBuffer) .setOperationProtobuf(ProtobufManager.getOrCreateOperation(invocation)); invocation.setTransportContext(transportContext); + invocation.mergeContext(header.getContext()); invocation.onStart(null, start); invocation.getInvocationStageTrace().startSchedule(); - // copied from HighwayCodec#decodeRequest() - // for temporary qps enhance purpose, we'll remove it when handler mechanism is refactored - invocation.mergeContext(header.getContext()); - Holder<Boolean> qpsFlowControlReject = checkQpsFlowControl(operationMeta); if (qpsFlowControlReject.value) { return; diff --git a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java index baaef46..ad8a29f 100644 --- a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java +++ b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java @@ -195,11 +195,9 @@ public class TestHighwayClient { Object result = doTestSend(vertx, pool, tcpClient, Response.ok("ok")); Assert.assertEquals("ok", result); - Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersRequest()); Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersResponse()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishClientFiltersResponse()); - Assert.assertEquals(nanoTime, invocationStageTrace.getStartSend()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishGetConnection()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishWriteToBuffer()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishReceiveResponse()); @@ -218,7 +216,6 @@ public class TestHighwayClient { Object result = doTestSend(vertx, pool, tcpClient, new InvocationException(Status.BAD_REQUEST, (Object) "failed")); Assert.assertEquals("failed", ((InvocationException) result).getErrorData()); - Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersRequest()); Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersResponse()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishClientFiltersResponse()); } @@ -239,7 +236,6 @@ public class TestHighwayClient { null); Assert.assertEquals("failed", ((InvocationException) result).getErrorData()); - Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersRequest()); Assert.assertEquals(nanoTime, invocationStageTrace.getStartClientFiltersResponse()); Assert.assertEquals(nanoTime, invocationStageTrace.getFinishClientFiltersResponse()); } 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 a990eca..a91b576 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 @@ -116,7 +116,9 @@ public class TestHighwayCodec { @Test - public void testDecodeRequestTraceId(@Mocked Endpoint endpoint) throws Exception { + public void test_decode_request_successful_and_not_copy_header(@Mocked Endpoint endpoint) throws Exception { + // test decode request not thrown exception and not copy header + // header should copied before invocation start. commonMock(); Invocation invocation = new Invocation(endpoint, operationMeta, null); @@ -132,7 +134,7 @@ public class TestHighwayCodec { context.put("X-B3-traceId", "test2"); HighwayCodec.decodeRequest(invocation, headers, operationProtobuf, bodyBuffer); - Assert.assertEquals("test2", invocation.getContext("X-B3-traceId")); + Assert.assertEquals("test1", invocation.getContext("X-B3-traceId")); } @Test diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSenderFilter.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSenderFilter.java index d5c2bd4..84e8baa 100644 --- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSenderFilter.java +++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSenderFilter.java @@ -38,6 +38,8 @@ public class RestClientSenderFilter implements ConsumerFilter { @Override public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode nextNode) { + invocation.onStartSendRequest(); + CompletableFuture<Response> future = new RestClientSender(invocation) .send(); diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java index 1cb5837..70a2ce7 100644 --- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java +++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java @@ -26,6 +26,7 @@ import org.apache.servicecomb.common.rest.RestConst; import org.apache.servicecomb.common.rest.codec.param.RestClientRequestImpl; import org.apache.servicecomb.common.rest.definition.RestOperationMeta; import org.apache.servicecomb.common.rest.filter.HttpClientFilter; +import org.apache.servicecomb.core.Const; import org.apache.servicecomb.core.Invocation; import org.apache.servicecomb.core.definition.OperationConfig; import org.apache.servicecomb.core.definition.OperationMeta; @@ -126,7 +127,7 @@ public class RestClientInvocation { }); // 从业务线程转移到网络线程中去发送 - invocation.getInvocationStageTrace().startSend(); + invocation.onStartSendRequest(); httpClientWithContext.runOnContext(httpClient -> { clientRequest.setTimeout(operationMeta.getConfig().getMsRequestTimeout()); processServiceCombHeaders(invocation, operationMeta); @@ -300,8 +301,7 @@ public class RestClientInvocation { protected void setCseContext() { try { - String cseContext = JsonUtils.writeValueAsString(invocation.getContext()); - clientRequest.putHeader(org.apache.servicecomb.core.Const.CSE_CONTEXT, cseContext); + clientRequest.putHeader(Const.CSE_CONTEXT, JsonUtils.writeValueAsString(invocation.getContext())); } catch (Throwable e) { invocation.getTraceIdLogger().error(LOGGER, "Failed to encode and set cseContext, message={}." , ExceptionUtils.getExceptionMessageWithoutTrace(e)); diff --git a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java index fd29183..dde6a22 100644 --- a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java +++ b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java @@ -191,7 +191,6 @@ public class TestRestClientInvocation { Assert.assertEquals(TARGET_MICROSERVICE_NAME, headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE)); Assert.assertEquals("{}", headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT)); Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersRequest()); - Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartSend()); } @Test @@ -207,7 +206,6 @@ public class TestRestClientInvocation { Assert.assertSame(resp, response); Assert.assertThat(headers.names(), Matchers.empty()); Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersRequest()); - Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartSend()); } @Test @@ -228,7 +226,6 @@ public class TestRestClientInvocation { Assert.assertEquals(TARGET_MICROSERVICE_NAME, headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE)); Assert.assertEquals("{}", headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT)); Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersRequest()); - Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartSend()); operationConfig.setClientRequestHeaderFilterEnabled(true); }