jolshan commented on a change in pull request #10282:
URL: https://github.com/apache/kafka/pull/10282#discussion_r593330038
##########
File path: core/src/main/scala/kafka/cluster/Partition.scala
##########
@@ -425,36 +425,35 @@ class Partition(val topicPartition: TopicPartition,
}
/**
- * Checks if the topic ID provided in the request is consistent with the
topic ID in the log.
+ * Checks if the topic ID received is consistent with the topic ID in the
log.
* If a valid topic ID is provided, and the log exists but has no ID set,
set the log ID to be the request ID.
*
- * @param requestTopicId the topic ID from the request
- * @return true if the request topic id is consistent, false otherwise
+ * @param receivedTopicId the topic ID from the LeaderAndIsr request or from
the metadata records
+ * @return true if the received topic id is consistent, false otherwise
*/
- def checkOrSetTopicId(requestTopicId: Uuid): Boolean = {
- // If the request had an invalid topic ID, then we assume that topic IDs
are not supported.
- // The topic ID was not inconsistent, so return true.
- // If the log is empty, then we can not say that topic ID is inconsistent,
so return true.
- if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID)
- true
- else {
+ def checkOrSetTopicId(receivedTopicId: Uuid, usingRaft: Boolean): Boolean = {
Review comment:
Hmm. Looking into this further, it seems that the log is created in
RaftReplicaManager when making a leader or a follower. This occurs in the
`handleMetadataRecords` method and the `endMetadataChangeDeferral` method.
Each requires a few methods to pass through before we get to
`LogManager.getOrCreateLog`, which is not ideal to pass a topic ID through
(especially when some of this code is shared with the ZK path, but not
impossible)
One thing I am wondering though is how I tested the changes. I think I
assume the log is created already, but this code suggests that the log will not
be created until `makeLeaders/Followers` in the non deferring case, and not
created until `endMetadataChangeDeferral` in the deferring case. This means
that where I set topic IDs now, we won't have a log on the first pass! So I
think maybe it is worth it to pass the ID through these methods. I think I can
keep the check of the topic ID where it is though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
