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]