You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/08/16 01:47:39 UTC
[servicecomb-java-chassis] branch master updated: [SCB-2299]improve
request write time calculation (#2501)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new b61a219 [SCB-2299]improve request write time calculation (#2501)
b61a219 is described below
commit b61a2199f333bd2780b514f82cf003c46bc99cd7
Author: liubao68 <bi...@qq.com>
AuthorDate: Mon Aug 16 09:47:30 2021 +0800
[SCB-2299]improve request write time calculation (#2501)
---
.../common/rest/codec/RestClientRequest.java | 3 +-
.../rest/codec/param/RestClientRequestImpl.java | 20 ++++-----
.../org/apache/servicecomb/core/Invocation.java | 48 +++++++++++-----------
.../core/invocation/InvocationStageTrace.java | 13 ++++--
.../core/invocation/TestInvocationStageTrace.java | 8 +++-
.../publish/TestInvocationPublishModelFactory.java | 1 +
.../transport/highway/HighwayClient.java | 8 ++--
.../transport/highway/HighwayClientFilter.java | 10 ++---
.../rest/client/RestClientCodecFilter.java | 8 +---
.../rest/client/RestClientRequestParameters.java | 3 +-
.../transport/rest/client/RestClientSender.java | 4 --
.../rest/client/RestClientSenderFilter.java | 2 +
.../rest/client/http/RestClientInvocation.java | 16 +++-----
.../rest/client/http/TestRestClientInvocation.java | 1 -
14 files changed, 72 insertions(+), 73 deletions(-)
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestClientRequest.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestClientRequest.java
index 73f7e4d..62d7688 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestClientRequest.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestClientRequest.java
@@ -17,6 +17,7 @@
package org.apache.servicecomb.common.rest.codec;
+import io.vertx.core.Future;
import io.vertx.core.MultiMap;
import io.vertx.core.buffer.Buffer;
@@ -27,7 +28,7 @@ import io.vertx.core.buffer.Buffer;
public interface RestClientRequest {
void write(Buffer bodyBuffer);
- void end();
+ Future<Void> end();
void addCookie(String name, String value);
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
index 118c53a..1280af7 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/RestClientRequestImpl.java
@@ -121,24 +121,23 @@ public class RestClientRequestImpl implements RestClientRequest {
}
@Override
- public void end() {
+ public Future<Void> end() {
writeCookies();
if (!uploads.isEmpty()) {
- doEndWithUpload();
- return;
+ return doEndWithUpload();
}
- doEndNormal();
+ return doEndNormal();
}
- protected void doEndWithUpload() {
+ protected Future<Void> doEndWithUpload() {
request.setChunked(true);
String boundary = "boundary" + UUID.randomUUID().toString();
putHeader(CONTENT_TYPE, MULTIPART_FORM_DATA + "; charset=UTF-8; boundary=" + boundary);
- genBodyForm(boundary).onSuccess(v -> attachFiles(boundary)).onFailure(e -> asyncResp.consumerFail(e));
+ return genBodyForm(boundary).onSuccess(v -> attachFiles(boundary)).onFailure(e -> asyncResp.consumerFail(e));
}
private Future<Void> genBodyForm(String boundary) {
@@ -169,20 +168,19 @@ public class RestClientRequestImpl implements RestClientRequest {
return string.getBytes(StandardCharsets.UTF_8);
}
- protected void doEndNormal() {
+ protected Future<Void> doEndNormal() {
try {
genBodyBuffer();
} catch (Exception e) {
asyncResp.consumerFail(e);
- return;
+ return Future.succeededFuture();
}
if (bodyBuffer == null) {
- request.end();
- return;
+ return request.end();
}
- request.end(bodyBuffer);
+ return request.end(bodyBuffer);
}
private void attachFiles(String boundary) {
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 30fe533..48e7747 100644
--- a/core/src/main/java/org/apache/servicecomb/core/Invocation.java
+++ b/core/src/main/java/org/apache/servicecomb/core/Invocation.java
@@ -117,30 +117,6 @@ public class Invocation extends SwaggerInvocation {
private Map<String, Object> swaggerArguments = Collections.emptyMap();
- public long getInvocationId() {
- return invocationId;
- }
-
- public TraceIdLogger getTraceIdLogger() {
- return this.traceIdLogger;
- }
-
- public HttpServletRequestEx getRequestEx() {
- return requestEx;
- }
-
- public InvocationStageTrace getInvocationStageTrace() {
- return invocationStageTrace;
- }
-
- public String getTraceId() {
- return getContext(Const.TRACE_ID_NAME);
- }
-
- public String getTraceId(String traceIdName) {
- return getContext(traceIdName);
- }
-
public Invocation() {
// An empty invocation, used to mock or some other scenario do not need operation information.
traceIdLogger = new TraceIdLogger(this);
@@ -476,6 +452,30 @@ public class Invocation extends SwaggerInvocation {
return referenceConfig.is3rdPartyService();
}
+ public long getInvocationId() {
+ return invocationId;
+ }
+
+ public TraceIdLogger getTraceIdLogger() {
+ return this.traceIdLogger;
+ }
+
+ public HttpServletRequestEx getRequestEx() {
+ return requestEx;
+ }
+
+ public InvocationStageTrace getInvocationStageTrace() {
+ return invocationStageTrace;
+ }
+
+ public String getTraceId() {
+ return getContext(Const.TRACE_ID_NAME);
+ }
+
+ public String getTraceId(String traceIdName) {
+ return getContext(traceIdName);
+ }
+
// ensure sync consumer invocation response flow not run in eventLoop
public <T> CompletableFuture<T> optimizeSyncConsumerThread(CompletableFuture<T> future) {
if (sync && !InvokerUtils.isInEventLoop()) {
diff --git a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationStageTrace.java b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationStageTrace.java
index 396a478..20029bc 100644
--- a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationStageTrace.java
+++ b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationStageTrace.java
@@ -100,6 +100,9 @@ public class InvocationStageTrace {
private long startSend;
// only for consumer
+ private long startGetConnection;
+
+ // only for consumer
private long finishGetConnection;
// only for consumer
@@ -200,8 +203,12 @@ public class InvocationStageTrace {
return finishGetConnection;
}
- public void finishGetConnection(long finishGetConnection) {
- this.finishGetConnection = finishGetConnection;
+ public void startGetConnection() {
+ this.startGetConnection = System.nanoTime();
+ }
+
+ public void finishGetConnection() {
+ this.finishGetConnection = System.nanoTime();
}
public long getFinishWriteToBuffer() {
@@ -325,7 +332,7 @@ public class InvocationStageTrace {
}
public double calcGetConnectionTime() {
- return calc(finishGetConnection, startSend);
+ return calc(finishGetConnection, startGetConnection);
}
public double calcWriteToBufferTime() {
diff --git a/core/src/test/java/org/apache/servicecomb/core/invocation/TestInvocationStageTrace.java b/core/src/test/java/org/apache/servicecomb/core/invocation/TestInvocationStageTrace.java
index 2a6a4b8..91c7dd9 100644
--- a/core/src/test/java/org/apache/servicecomb/core/invocation/TestInvocationStageTrace.java
+++ b/core/src/test/java/org/apache/servicecomb/core/invocation/TestInvocationStageTrace.java
@@ -74,8 +74,10 @@ public class TestInvocationStageTrace {
nanoTime = 3;
stageTrace.startClientFiltersRequest();
nanoTime = 4;
+ stageTrace.startGetConnection();
stageTrace.startSend();
- stageTrace.finishGetConnection(5);
+ nanoTime = 5;
+ stageTrace.finishGetConnection();
stageTrace.finishWriteToBuffer(6);
nanoTime = 7;
stageTrace.finishReceiveResponse();
@@ -178,8 +180,10 @@ public class TestInvocationStageTrace {
nanoTime = 6;
stageTrace.startClientFiltersRequest();
nanoTime = 7;
+ stageTrace.startGetConnection();
stageTrace.startSend();
- stageTrace.finishGetConnection(8);
+ nanoTime = 8;
+ stageTrace.finishGetConnection();
stageTrace.finishWriteToBuffer(9);
nanoTime = 10;
stageTrace.finishReceiveResponse();
diff --git a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/publish/TestInvocationPublishModelFactory.java b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/publish/TestInvocationPublishModelFactory.java
index 2f7dbc7..98a75a6 100644
--- a/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/publish/TestInvocationPublishModelFactory.java
+++ b/metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/publish/TestInvocationPublishModelFactory.java
@@ -333,6 +333,7 @@ public class TestInvocationPublishModelFactory {
Deencapsulation.setField(invocationStageTrace, "startHandlersRequest", 2L);
Deencapsulation.setField(invocationStageTrace, "startClientFiltersRequest", 3L);
Deencapsulation.setField(invocationStageTrace, "startSend", 4L);
+ Deencapsulation.setField(invocationStageTrace, "startGetConnection", 4L);
Deencapsulation.setField(invocationStageTrace, "finishGetConnection", 5L);
Deencapsulation.setField(invocationStageTrace, "finishWriteToBuffer", 6L);
Deencapsulation.setField(invocationStageTrace, "finishReceiveResponse", 7L);
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 cbf233a..d19e8b0 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
@@ -86,14 +86,14 @@ public class HighwayClient {
}
public void send(Invocation invocation, AsyncResponse asyncResp) throws Exception {
- invocation.getInvocationStageTrace().startClientFiltersRequest();
- invocation.onStartSendRequest();
-
+ invocation.getInvocationStageTrace().startGetConnection();
HighwayClientConnection tcpClient = findClientPool(invocation);
+ invocation.getInvocationStageTrace().startClientFiltersRequest();
OperationProtobuf operationProtobuf = ProtobufManager.getOrCreateOperation(invocation);
HighwayClientPackage clientPackage = createClientPackage(invocation, operationProtobuf);
+ invocation.onStartSendRequest();
tcpClient.send(clientPackage, ar -> {
invocation.getInvocationStageTrace().finishWriteToBuffer(clientPackage.getFinishWriteToBuffer());
invocation.getInvocationStageTrace().finishReceiveResponse();
@@ -142,7 +142,7 @@ public class HighwayClient {
HighwayClientConnection tcpClient = clientMgr.findClientPool(invocation.isSync())
.findOrCreateClient(invocation.getEndpoint().getEndpoint());
- invocation.getInvocationStageTrace().finishGetConnection(System.nanoTime());
+ invocation.getInvocationStageTrace().finishGetConnection();
return tcpClient;
}
diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClientFilter.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClientFilter.java
index ccebe79..77d17a4 100644
--- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClientFilter.java
+++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClientFilter.java
@@ -52,8 +52,6 @@ public class HighwayClientFilter implements ConsumerFilter {
invocation.getMicroserviceQualifiedName(),
invocation.getEndpoint().getEndpoint());
- invocation.getInvocationStageTrace().startClientFiltersRequest();
-
OperationProtobuf operationProtobuf = ProtobufManager.getOrCreateOperation(invocation);
return send(invocation, operationProtobuf)
.thenApply(tcpData -> convertToResponse(invocation, operationProtobuf, tcpData))
@@ -62,11 +60,13 @@ public class HighwayClientFilter implements ConsumerFilter {
}
protected CompletableFuture<TcpData> send(Invocation invocation, OperationProtobuf operationProtobuf) {
- invocation.onStartSendRequest();
-
+ invocation.getInvocationStageTrace().startGetConnection();
HighwayClient highwayClient = ((HighwayTransport) invocation.getTransport()).getHighwayClient();
HighwayClientPackage clientPackage = highwayClient.createClientPackage(invocation, operationProtobuf);
- CompletableFuture<TcpData> sendFuture = highwayClient.findClientPool(invocation)
+ HighwayClientConnection clientConnection = highwayClient.findClientPool(invocation);
+ invocation.getInvocationStageTrace().startClientFiltersRequest();
+ invocation.onStartSendRequest();
+ CompletableFuture<TcpData> sendFuture = clientConnection
.send(clientPackage)
.whenComplete((tcpData, throwable) -> afterSend(invocation, clientPackage));
return invocation.optimizeSyncConsumerThread(sendFuture);
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
index 6fdd3cd..6a530f2 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
@@ -65,10 +65,8 @@ public class RestClientCodecFilter implements ConsumerFilter {
@Override
public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode nextNode) {
+ invocation.getInvocationStageTrace().startGetConnection();
startClientFiltersRequest(invocation);
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
- invocation.onStartSendRequest();
- //
return CompletableFuture.completedFuture(null)
.thenCompose(v -> transportContextFactory.createHttpClientRequest(invocation).toCompletionStage())
.thenAccept(httpClientRequest -> prepareTransportContext(invocation, httpClientRequest))
@@ -83,9 +81,7 @@ public class RestClientCodecFilter implements ConsumerFilter {
}
protected void prepareTransportContext(Invocation invocation, HttpClientRequest httpClientRequest) {
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
- invocation.getInvocationStageTrace().finishGetConnection(System.nanoTime());
- //
+ invocation.getInvocationStageTrace().finishGetConnection();
RestClientTransportContext transportContext = transportContextFactory.create(invocation, httpClientRequest);
invocation.setTransportContext(transportContext);
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientRequestParameters.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientRequestParameters.java
index 72aa232..964cf5f 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientRequestParameters.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientRequestParameters.java
@@ -24,6 +24,7 @@ import org.apache.servicecomb.common.rest.codec.RestClientRequest;
import com.google.common.collect.Multimap;
+import io.vertx.core.Future;
import io.vertx.core.buffer.Buffer;
public interface RestClientRequestParameters extends RestClientRequest {
@@ -44,7 +45,7 @@ public interface RestClientRequestParameters extends RestClientRequest {
}
@Override
- default void end() {
+ default Future<Void> end() {
throw new UnsupportedOperationException("should not invoke this method");
}
}
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
index c93ef16..1c11a04 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
@@ -172,11 +172,7 @@ public class RestClientSender {
protected void processMetrics() {
InvocationStageTrace stageTrace = invocation.getInvocationStageTrace();
-
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
stageTrace.finishWriteToBuffer(System.nanoTime());
- //
-
// even failed and did not received response, still set time for it
// that will help to know the real timeout time
stageTrace.finishReceiveResponse();
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 279b281..72252e9 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
@@ -102,11 +102,9 @@ public class RestClientInvocation {
Future<HttpClientRequest> requestFuture = createRequest(ipPort, path);
- invocation.onStartSendRequest();
+ invocation.getInvocationStageTrace().startGetConnection();
requestFuture.compose(clientRequest -> {
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
- invocation.getInvocationStageTrace().finishGetConnection(System.nanoTime());
- //
+ invocation.getInvocationStageTrace().finishGetConnection();
this.clientRequest = clientRequest;
@@ -130,6 +128,7 @@ public class RestClientInvocation {
}
// 从业务线程转移到网络线程中去发送
+ invocation.onStartSendRequest();
httpClientWithContext.runOnContext(httpClient -> {
clientRequest.setTimeout(operationMeta.getConfig().getMsRequestTimeout());
clientRequest.response().onComplete(asyncResult -> {
@@ -140,7 +139,8 @@ public class RestClientInvocation {
handleResponse(asyncResult.result());
});
processServiceCombHeaders(invocation, operationMeta);
- restClientRequest.end();
+ restClientRequest.end()
+ .onComplete((t) -> invocation.getInvocationStageTrace().finishWriteToBuffer(System.nanoTime()));
});
return Future.succeededFuture();
}).onFailure(failure -> {
@@ -229,10 +229,6 @@ public class RestClientInvocation {
* @param responseBuf response body buffer, when download, responseBuf is null, because download data by ReadStreamPart
*/
protected void processResponseBody(Buffer responseBuf) {
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
- invocation.getInvocationStageTrace().finishWriteToBuffer(System.nanoTime());
- //
-
invocation.getInvocationStageTrace().finishReceiveResponse();
invocation.getResponseExecutor().execute(() -> {
try {
@@ -268,11 +264,9 @@ public class RestClientInvocation {
InvocationStageTrace stageTrace = invocation.getInvocationStageTrace();
- // TODO: after upgrade vert.x , can use request metric to calculate request end time
if (stageTrace.getFinishWriteToBuffer() == 0) {
stageTrace.finishWriteToBuffer(System.nanoTime());
}
- //
// even failed and did not received response, still set time for it
// that will help to know the real timeout time
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 4d78204..34a19fe 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
@@ -324,7 +324,6 @@ public class TestRestClientInvocation {
Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersResponse());
Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishClientFiltersResponse());
Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishReceiveResponse());
- Assert.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishWriteToBuffer());
}
@SuppressWarnings("unchecked")