dajac commented on a change in pull request #8933: URL: https://github.com/apache/kafka/pull/8933#discussion_r456266252
########## File path: clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java ########## @@ -97,7 +97,25 @@ public static RecordingLevel forName(String name) { public boolean shouldRecord(final int configId) { return configId == DEBUG.id || configId == this.id; } + } + public enum QuotaEnforcementType { Review comment: I think that we do. I actually did exactly what you suggest in the first place but then realised that it introduces a race condition. For instance, two requests processed in two separate threads can verify the quota at the same time. If it is below the authorized quota, they would go ahead, record the value, and do the creation/deletion. That violates the strict enforcement of the quota as we don't want to accept a creation/deletion if the quota is already violated. In this case, the first request must succeed while the second must fail. The proposed approach allows to do the quota verification and the recording within the sensor lock. That basically allows us to record the value iif the quota is not already violated. This is probably less an issue for our existing quotas because we always accept requests, even if the quota is already violated. Regarding the naming, we may find better ones. Would you have any suggestions? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org