[GitHub] [pulsar] codelipenghui commented on a diff in pull request #20568: [improve] [broker] Close connection if got an IO error when sending messages to the client

["codelipenghui (via GitHub)" <gi...@apache.org>]

codelipenghui commented on code in PR #20568:
URL: https://github.com/apache/pulsar/pull/20568#discussion_r1243507440


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -3417,6 +3429,17 @@ public CompletableFuture<Boolean> checkConnectionLiveness() {
         }
     }
 
+    @Override
+    public void healthCheckManually() {
+        long lastCheckTime = LAST_MANUAL_HEARTBEAT_CHECK_TIME_UPDATER.get(this);
+        if (System.currentTimeMillis() - lastCheckTime < 5000) {
+            return;
+        }
+        if (LAST_MANUAL_HEARTBEAT_CHECK_TIME_UPDATER.compareAndSet(this, lastCheckTime, System.currentTimeMillis())) {
+            sendPing();

Review Comment:
   I'm not sure how it can resolve the problem. If the ping command is sent, but no pong response from the client side, does the broker need to wait for the keep-alive timeout? But the keep alive task already handled it, right?
   
   Or maybe we should call `handleKeepAliveTimeout()` so that the connection can be closed forcelly without waiting for the response.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java:
##########
@@ -361,6 +361,12 @@ public Future<Void> sendMessages(final List<? extends Entry> entries, EntryBatch
                 msgOutCounter.add(totalMessages);
                 bytesOutCounter.add(totalBytes);
                 chunkedMessageRate.recordMultipleEvents(totalChunkedMessages, 0);
+            } else {
+                log.warn("[{}-{}] Sent messages to client fail by IO exception[{}], these messages(messages count:"

Review Comment:
   Is it better to change the log level to debug? It's a message level log, for a cluster with high consumption throughput, it will introduce too many warning logs. We should be careful to add logs on the message level with `>=` info level.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org