[GitHub] [kafka] showuon commented on a diff in pull request #12590: KAFKA-7109: Close fetch sessions on close of consumer

[GitBox]

showuon commented on code in PR #12590:
URL: https://github.com/apache/kafka/pull/12590#discussion_r967815118


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java:
##########
@@ -446,6 +429,31 @@ private RequestFuture<ClientResponse> 
sendMetadataRequest(MetadataRequest.Builde
             return client.send(node, request);
     }
 
+    /**
+     * Send Fetch Request to Kafka cluster asynchronously.
+     *
+     * This method is visible for testing.
+     *
+     * @return A future that indicates result of sent Fetch request
+     */
+    RequestFuture<ClientResponse> sendFetchRequestToNode(final 
FetchSessionHandler.FetchRequestData requestData,
+                                                                 final Node 
fetchTarget) {

Review Comment:
   nit: indent



##########
clients/src/main/java/org/apache/kafka/common/requests/FetchMetadata.java:
##########
@@ -114,9 +114,21 @@ public boolean equals(Object o) {
     }
 
     /**
-     * Return the metadata for the next error response.
+     * Return the metadata for the next request. The metadata is set to 
indicate that the client wants to close the
+     * existing session.
      */
     public FetchMetadata nextCloseExisting() {
+        return new FetchMetadata(sessionId, FINAL_EPOCH);

Review Comment:
   Added the last row. We don't need another poll because we didn't change 
anything. We just added missing part in the KIP.
   
   <img width="1495" alt="image" 
src="https://user-images.githubusercontent.com/43372967/189527055-936c4f46-18be-4b81-9874-a15ccb836196.png";>
   



-- 
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: jira-unsubscr...@kafka.apache.org

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