junrao commented on code in PR #14699:
URL: https://github.com/apache/kafka/pull/14699#discussion_r1397714746
##########
clients/src/main/java/org/apache/kafka/common/requests/PushTelemetryRequest.java:
##########
@@ -60,17 +62,47 @@ public PushTelemetryRequest(PushTelemetryRequestData data,
short version) {
@Override
public PushTelemetryResponse getErrorResponse(int throttleTimeMs,
Throwable e) {
- PushTelemetryResponseData responseData = new
PushTelemetryResponseData()
- .setErrorCode(Errors.forException(e).code())
- .setThrottleTimeMs(throttleTimeMs);
- return new PushTelemetryResponse(responseData);
+ return errorResponse(throttleTimeMs, Errors.forException(e));
}
@Override
public PushTelemetryRequestData data() {
return data;
}
+ public PushTelemetryResponse errorResponse(int throttleTimeMs, Errors
errors) {
+ PushTelemetryResponseData responseData = new
PushTelemetryResponseData();
+ responseData.setErrorCode(errors.code());
+ /*
+ THROTTLING_QUOTA_EXCEEDED is thrown in telemetry APIs when the
telemetry request
+ arrives prior the configured minimum interval between two consecutive
telemetry requests.
+ In this case, the throttleTimeMs should not be included in the
response to avoid the
+ muting of the client channel for all the other requests.
+ */
+ if (Errors.THROTTLING_QUOTA_EXCEEDED != errors) {
Review Comment:
I noticed that sometimes we do set `throttleTimeMs` with an error. However,
this only happens in KafkaApis with the generic request throttling logic. So,
for now, I'd recommend that we get rid of this check here and rely on the
caller to set `throttleTimeMs` properly. Specifically, for all
`errorResponse(int throttleTimeMs, Errors errors)` and `getErrorResponse(int
throttleTimeMs, Throwable e)` calls in `ClientMetricsManager`, we should pass
in 0 for `throttleTimeMs`.
--
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
