[#4477]Fix GoAway issue in Http2 after many requests timeout (#4487)

apache · Sep 2, 2024 · 06ff2f0 · 06ff2f0
1 parent 0863a3b
commit 06ff2f0
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 11 deletions.
diff --git a/.../java/org/apache/servicecomb/common/accessLog/core/element/impl/HttpStatusAccessItem.java b/.../java/org/apache/servicecomb/common/accessLog/core/element/impl/HttpStatusAccessItem.java
@@ -21,15 +21,11 @@
 import org.apache.servicecomb.core.event.InvocationFinishEvent;
 import org.apache.servicecomb.core.event.ServerAccessLogEvent;
 import org.apache.servicecomb.swagger.invocation.Response;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import io.vertx.core.http.HttpServerResponse;
 import io.vertx.ext.web.RoutingContext;
 
 public class HttpStatusAccessItem implements AccessLogItem<RoutingContext> {
-  private static final Logger LOGGER = LoggerFactory.getLogger(HttpStatusAccessItem.class);
-
   public static final String EMPTY_RESULT = "-";
 
   @Override
@@ -40,9 +36,6 @@ public void appendServerFormattedItem(ServerAccessLogEvent accessLogEvent, Strin
       return;
     }
     if (response.closed() && !response.ended()) {
-      LOGGER.warn(
-          "Response is closed before sending any data. "
-              + "Please check idle connection timeout for provider is properly configured.");
       builder.append(EMPTY_RESULT);
       return;
     }

diff --git a/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java b/core/src/main/java/org/apache/servicecomb/core/invocation/ProducerInvocationFlow.java
@@ -72,7 +72,9 @@ private void tryRunInvocation(Invocation invocation) {
             if (throwable != null) {
               // Server codec operates on Response. So the filter chain result should be Response and
               // will never throw exception.
-              LOGGER.error("Maybe a fatal bug that should be addressed.", throwable);
+              // Sometimes Server Codec will throw exception, e.g. Connection Closed.
+              LOGGER.error("Maybe a fatal bug that should be addressed or codec exception for {}.",
+                  invocation.getInvocationQualifiedName(), throwable);
               response = Response.createFail(new InvocationException(Status.INTERNAL_SERVER_ERROR,
                   new CommonExceptionData("Internal error, check logs for details.")));
             }
@@ -87,7 +89,9 @@ private void tryRunInvocation(Invocation invocation) {
           if (throwable != null) {
             // Server codec operates on Response. So the filter chain result should be Response and
             // will never throw exception.
-            LOGGER.error("Maybe a fatal bug that should be addressed.", throwable);
+            // Sometimes Server Codec will throw exception, e.g. Connection Closed.
+            LOGGER.error("Maybe a fatal bug that should be addressed or codec exception for {}.",
+                invocation.getInvocationQualifiedName(), throwable);
             response = Response.createFail(new InvocationException(Status.INTERNAL_SERVER_ERROR,
                 new CommonExceptionData("Internal error, check logs for details.")));
           }

diff --git a/...-zookeeper/provider/src/main/java/org/apache/servicecomb/samples/WebsocketController.java b/...-zookeeper/provider/src/main/java/org/apache/servicecomb/samples/WebsocketController.java
@@ -33,14 +33,24 @@ public class WebsocketController {
   @PostMapping("/websocket")
   @Transport(name = CoreConst.WEBSOCKET)
   public void websocket(ServerWebSocket serverWebsocket) {
+    // Client may have not registered message handler, and messages sent may get lost.
+    // So we sleep for a while to send message.
     AtomicInteger receiveCount = new AtomicInteger(0);
-    serverWebsocket.writeTextMessage("hello", r -> {
-    });
     serverWebsocket.textMessageHandler(s -> {
       receiveCount.getAndIncrement();
     });
     serverWebsocket.closeHandler((v) -> System.out.println("closed"));
+
     new Thread(() -> {
+      try {
+        Thread.sleep(1000);
+      } catch (InterruptedException e) {
+        e.printStackTrace();
+      }
+
+      serverWebsocket.writeTextMessage("hello", r -> {
+      });
+
       for (int i = 0; i < 5; i++) {
         serverWebsocket.writeTextMessage("hello " + i, r -> {
         });

diff --git a/...rg/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java b/...rg/apache/servicecomb/foundation/vertx/http/VertxServerResponseToHttpServletResponse.java
@@ -17,6 +17,7 @@
 
 package org.apache.servicecomb.foundation.vertx.http;
 
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
@@ -116,6 +117,9 @@ public void endResponse() {
   }
 
   public void internalFlushBuffer() {
+    if (serverResponse.closed()) {
+      return;
+    }
     serverResponse.end();
   }
 
@@ -130,6 +134,10 @@ public CompletableFuture<Void> sendPart(Part part) {
 
   @Override
   public CompletableFuture<Void> sendBuffer(Buffer buffer) {
+    if (serverResponse.closed()) {
+      return CompletableFuture.failedFuture(new IOException("Response is closed before sending any data. "
+          + "Maybe client is timeout or check idle connection timeout for provider is properly configured."));
+    }
     CompletableFuture<Void> future = new CompletableFuture<>();
     serverResponse.write(buffer).onComplete(result -> {
       if (result.failed()) {