divijvaidya commented on code in PR #14629:
URL: https://github.com/apache/kafka/pull/14629#discussion_r1376654240
##########
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:
> actionable practical recommendation
I can't think of an idea other than passing deep copy of arguments. I have
also considered suggesting wrapping in Collections.unmodifiable or
Collections.synchronizedCollection but none of these ideas are good enough.
We need to re-think callback invocation pattern in Kafka and see if we can
avoid this pattern of passing reference of in-memory state around threads. But
I am happy to keep it our of this PR and discuss that in scope of JIRA that
Ismael created.
--
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]
