splett2 commented on code in PR #14053:
URL: https://github.com/apache/kafka/pull/14053#discussion_r1324845475
##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -137,7 +137,8 @@ object Partition {
delayedOperations = delayedOperations,
metadataCache = replicaManager.metadataCache,
logManager = replicaManager.logManager,
- alterIsrManager = replicaManager.alterPartitionManager)
+ alterIsrManager = replicaManager.alterPartitionManager,
+ zkMigrationEnabled = () => replicaManager.config.migrationEnabled)
Review Comment:
we can just pass this in as a simple boolean. My understanding is that zk
migration is not a dynamic config.
##########
core/src/main/scala/kafka/cluster/Replica.scala:
##########
@@ -98,14 +103,22 @@ class Replica(val brokerId: Int, val topicPartition:
TopicPartition) extends Log
* fetch request is always smaller than the leader's LEO, which can happen
if small produce requests are received at
* high frequency.
*/
- def updateFetchState(
+ def updateFetchStateOrThrow(
followerFetchOffsetMetadata: LogOffsetMetadata,
followerStartOffset: Long,
followerFetchTimeMs: Long,
leaderEndOffset: Long,
- brokerEpoch: Long
+ brokerEpoch: Long,
+ verifyBrokerEpoch: Boolean = false
Review Comment:
I would prefer not to use a default parameter. I guess we did it because
`updateFetchStateOrThrow` has quite a few callers.
Maybe we can pass the value for `verifyBrokerEpoch` into the `Replica`
constructor to minimize some of the changes? It also helps us keep the
`updateFetchState` a bit simpler.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
