jolshan commented on code in PR #14629:
URL: https://github.com/apache/kafka/pull/14629#discussion_r1380898562
##########
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:
I was saying that appendEntries queues up a log record and we could use it
there.
I think Artem was not necessarily disagreeing with that idea but rather that
a lock should be used at all. My understanding of his comment was that we
should move to a model where changes are pending and those state transitions
block further work.
I think we also discussed that the ordering of the log is not the important
part but rather committing stale data. It was unclear to me from Jason's
comments if this validation is fully necessary due to how we can sometimes
write stale data to an offset partition when a rebalance happens after offsets
are are written as part of the transaction but before the transaction is
committed.
Basically there are two questions I see:
1. Can we make this code more async friendly to avoid locking? (And does the
new group coordinator do this?)
2. Do we really need the lock if we can write inconsistent data outside the
typical locking mechanisms and we handle it correctly in those cases?
--
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]
