[GitHub] [kafka] soarez commented on a diff in pull request #12314: KAFKA-13903: Queue size metric in QuorumController

[GitBox <gi...@apache.org>]

soarez commented on code in PR #12314:
URL: https://github.com/apache/kafka/pull/12314#discussion_r902056110


##########
server-common/src/main/java/org/apache/kafka/queue/KafkaEventQueue.java:
##########
@@ -178,15 +190,18 @@ public void run() {
         }
 
         private void remove(EventContext eventContext) {
-            eventContext.remove();
+            boolean removed = eventContext.remove();
             if (eventContext.deadlineNs.isPresent()) {
-                deadlineMap.remove(eventContext.deadlineNs.getAsLong());
+                removed |= deadlineMap.remove(eventContext.deadlineNs.getAsLong()) != null;
                 eventContext.deadlineNs = OptionalLong.empty();
             }
             if (eventContext.tag != null) {
                 tagToEventContext.remove(eventContext.tag, eventContext);
                 eventContext.tag = null;
             }
+            if (removed) {
+                onQueueSizeChange.accept(size.decrementAndGet());

Review Comment:
   The goal isn't to have an arbitrary consumer, we just need to propagate the changes to `ControllerMetrics`. I'm not sure what you mean with "an async explicit metric publish event", but I agree it makes senses that we should be careful while holding the lock. I think we can invert the logic and instead of pushing the size change to `ControllerMetrics`, we can use a Supplier instead to pull it.



-- 
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