[GitHub] [kafka] cmccabe commented on pull request #13116: KAFKA-14351: Controller Mutation Quota for KRaft

["cmccabe (via GitHub)" <gi...@apache.org>]

cmccabe commented on PR #13116:
URL: https://github.com/apache/kafka/pull/13116#issuecomment-1457277048

   Thanks for the PR, @rondagostino . You've been very patient here and I'm sorry that the review wasn't quicker. We did a major pivot from implementing the quota in `ControllerApis` like I originally wanted, to implementing it in `QuorumController` like you originally wanted. And I think the QC way is the way it has to be, for all the reasons discussed above. So you were right all along :)
   
   I spent a few hours looking through this code today to see if there were any obvious ways to improve the performance characteristics. The main thing I worry about is the impact of locking. The locking in `StrictControllerMutationQuota.record`kj is certainly frustrating, since I could see it colliding with metrics collection. There are several small locks that might be contended (sensor locks, KafkaMetric locks). None of them seems to be held for a very long time, though. So overall I cannot find any easy way to avoid this worry. We will have to benchmark, I guess.
   
   I left some minor comments that probably you can do in a day or two. I think we're pretty close now.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org