junrao commented on code in PR #14629:
URL: https://github.com/apache/kafka/pull/14629#discussion_r1380888754
##########
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:
Yes, the current flow in group coordinator is updating the state, queuing up
a corresponding log record for commit, and calling the callback when the log
record is committed. It needs the log records for a group to be queued up and
committed in the same order as the updating of the state so that (1) the latest
state can be replayed from the log records correctly in case of failure and (2)
the callback can be called in the updating order. Currently, this is achieved
through holding the group lock during updating the state and queuing up of the
log record (implemented through `ReplicaManager.append`). The expectation is
that queuing up of the log record can be done synchronously. Adding async txn
validation dependency inside `ReplicaManager.append` breaks that expectation.
To address this issue, we could move the async txn validation dependency out
of `ReplicaManager.append`. Alternatively, we could implement another way of
synchronously queuing up a log record since `ReplicaManager.append` does more
than just queuing up a record.
I am not sure holding the group lock just for updating the state is enough
for ensuring the ordering of the log records.
--
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]
