Hi all, I was looking through the code related to "dynamic Log config change" feature and noticed the way we deal with concurrency there. I have a question about it.
The Log class holds volatile LogConfig property, almost all methods in Log.scala are synchronized on private lock object. But the code in TopicConfigManager ( https://github.com/apache/kafka/blob/7130da90a9ee9e6fb4beb2a2a6ab05c06c9bfac4/core/src/main/scala/kafka/server/TopicConfigManager.scala#L108 ) which "substitutes" Log's logConfig is not synchronized. Code execution example: Thread 1: Log.append -> Log:288 config.*maxMessageSize* is accessed https://github.com/apache/kafka/blob/7130da90a9ee9e6fb4beb2a2a6ab05c06c9bfac4/core/src/main/scala/kafka/log/Log.scala#L288 Thread 2: handles log config change -> TopicConfigManager:108 (see above) substitutes log's config - changes *maxMessageSize* and *segmentSize* Thread 1: Log.append Log:299 - code accesses config.*segmentSize* and pickups updated config setting https://github.com/apache/kafka/blob/7130da90a9ee9e6fb4beb2a2a6ab05c06c9bfac4/core/src/main/scala/kafka/log/Log.scala#L299 So looks like we accessed object in partial "state" - in scope of one procedure (Log.append) we took one setting from the old state (maxMessageSize), and the other one from the updated state. Methods in Log are synchronized, as mentioned above. But logConfig is only volatile which solves visibility problems but doesn't prevent it from being changed in other thread, as I understand. Am I missing something here? Thanks, Andrii Biletskyi
