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


Reply via email to