cmccabe commented on code in PR #13116: URL: https://github.com/apache/kafka/pull/13116#discussion_r1091311692
########## core/src/main/scala/kafka/server/ControllerApis.scala: ########## @@ -322,15 +325,37 @@ class ControllerApis(val requestChannel: RequestChannel, } } // Finally, the idToName map contains all the topics that we are authorized to delete. - // Perform the deletion and create responses for each one. - controller.deleteTopics(context, idToName.keySet).thenApply { idToError => - idToError.forEach { (id, error) => - appendResponse(idToName.get(id), id, error) + // First check the controller mutation quota if necessary, and then Review Comment: This is a lot more logic (and expense) than what I wanted, and unfortunately it also has a TOCTOU, because partitions may be added in between checking here and doing the deletion. I guess I simply didn't forsee this, sorry. I wonder if we could convert the throttle to be lockless so that we could use it in the QuorumController thread itself. Like I said earlier, the big concern is that blocking the controller thread is really bad and not something we want to do under really any condition. It seems like the throttle should be simple... just a number or something right? Or can we somehow guarantee that only the controller thread itself is taking that lock under ordinary conditions? -- 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