[GitHub] [kafka] divijvaidya commented on pull request #12281: KAFKA-13971: Fix atomicity violations caused by improper usage of ConcurrentHashMap - part2

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

divijvaidya commented on PR #12281:
URL: https://github.com/apache/kafka/pull/12281#issuecomment-1152985895

   > Is the relevant code specified as thread safe?
   
   Thank you for your review @ijuma. I appreciate it. Though, I am afraid I don't understand your question. 
   
   Are you asking whether the existing code is supposed to be thread safe? 
   If yes, for `DelegatingClassLoader.java` the javadoc for the class mentioned that it is supposed to be thread safe (but it isn't due to the bug that is fixed in this review). For the `RocksDBMetricsRecordingTrigger.java`, we run a thread periodically from a metric trigger thread pool which reads from the map maintained in the class. At the same time it is possible that another thread is mutating the map during startup/shutdown of rocksDB which may leave the map in inconsistent state. Hence, it's important for this class to be thread safe as well. 
   Also, note that both the classes in this review use ConcurrentHashMap (albeit incorrectly) to ensure thread safe mutation over the map.
   
   Are you asking whether the changed code is thread safe?
   If yes, the change uses atomic operations provided by ConcurrentHashMap to ensure thread safety.


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