jolshan commented on code in PR #14629:
URL: https://github.com/apache/kafka/pull/14629#discussion_r1376481605
##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -864,6 +778,111 @@ class ReplicaManager(val config: KafkaConfig,
}
}
+ /*
+ * Note: This method can be used as a callback in a different request
thread. Ensure that correct RequestLocal
+ * is passed when executing this method. Accessing non-thread-safe data
structures should be avoided if possible.
+ */
+ private def appendEntries(allEntries: Map[TopicPartition, MemoryRecords],
+ internalTopicsAllowed: Boolean,
+ origin: AppendOrigin,
+ requiredAcks: Short,
+ verificationGuards: Map[TopicPartition,
VerificationGuard],
Review Comment:
Thanks for the discussion here folks. My point about the immutability of the
map is that we can't put new objects in the map and the values appeared to be
final. I agree that the method itself (and the argument for wrap) doesn't
guarantee immutability and/or thread safety.
I think we can agree the the RequestLocal object provides specific
challenges for callbacks that require it. Specifically callbacks that may
execute on a different thread may see issues.
How do we want to move forward from here? I can modify my comment and the
class to remove the comments about thread safety etc and focus on the fact that
the method may execute on a different thread. (ie name the class something that
reflects that) Were there any other changes we think are necessary?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
