divijvaidya commented on PR #13078: URL: https://github.com/apache/kafka/pull/13078#issuecomment-1410523645
@anatasiavela the proposal (using concurrentHashMap for producers) impacts this change. Let me try to explain why. Currently you are using `producerIdCount` and updating it on every add/delete to the `producers` map. Alternative path was to simply call `producers.size()` whenever we want to compute the metric. But you didn't choose this path because `producers` is accessed by multiple threads at the same time and we usually guard access to it via a lock. Hence, if you wanted to call `producers.size()` you would have to acquire a lock and that is not optimal for a mere metric calculation. Hence, your approach made sense. BUT if you change the `producers` to a concurrentHashMap, you wouldn't have to acquire a lock to call `producers.size()` when computing for metrics. Without acquiring a lock, the risk is that another thread may be mutating the `producers` map at the same time. As per the discussion above in the PR, that is acceptable to us. Hence, you can simply change the `producers` to a concurrentHashMap and remove the code logic to update `producerIdCount` on every add/remove. This simplifi es the code greatly. Let me know if that makes sense. -- 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
