chia7712 commented on code in PR #23094:
URL: https://github.com/apache/kafka/pull/23094#discussion_r3726299533


##########
server/src/main/java/org/apache/kafka/server/ClientMetricsManager.java:
##########
@@ -204,11 +204,13 @@ public PushTelemetryResponse 
processPushTelemetryRequest(PushTelemetryRequest re
             log.debug("Error validating push telemetry request from client 
[{}]", clientInstanceId, exception);
             clientInstance.lastKnownError(Errors.forException(exception));
             return request.getErrorResponse(0, exception);
-        } finally {
-            // Update the client instance with the latest push request 
parameters.
-            clientInstance.terminating(request.data().terminating());
         }
 
+        // Update the client instance with the latest push request parameters 
only

Review Comment:
   The reported edge case is specifically about the `subscriptionId` changing, 
so should we only skip setting the terminating flag for the ID check? I agree 
it's more consistent to include all validation checks, but I wanted to raise 
this for discussion



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to