rondagostino commented on a change in pull request #10069:
URL: https://github.com/apache/kafka/pull/10069#discussion_r572442503
##########
File path: core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
##########
@@ -39,11 +41,30 @@ object MetadataPartition {
record.isr(),
Collections.emptyList(), // TODO KAFKA-12285 handle offline replicas
Collections.emptyList(),
- Collections.emptyList())
+ Collections.emptyList(),
+ largestDeferredOffsetEverSeen =
deferredAtOffset.getOrElse(OffsetNeverDeferred),
+ isCurrentlyDeferringChanges = deferredAtOffset.isDefined)
Review comment:
We basically use `largestDeferredOffsetEverSeen` only for logging at
this point -- we also check it in a few `private def sanityCheckState...()`
`RaftReplicaManager` methods. We could completely eliminate
`largestDeferredOffsetEverSeen` if we didn't want to log when the partition was
last deferred. It just tracks when the partition was last seen and the change
at that offset was deferred rather than directly applied. Once the partition
is no longer deferred the value remains whatever it was and the boolean flips
to `false`.
It does seem on the surface that we could change the declaration to
`deferredSinceOffset` and get rid of the boolean -- and `deferredSinceOffset`
would change to `-1` once those changes are applied. But there is a problem
with this if the partition changes to not being deferred in the metadata cache
before we ask `RaftReplicaManager` to process all of its deferred changes: the
value will be -1 in the metadata cache under those circumstances, and we
wouldn't have the value to log.
So I think we have a few options.
1. Do the logging, apply the changes to the matadata cache before replica
manager, and keep the `Long` and `Boolean` as currently defined
2. Do the logging, apply the changes to the matadata cache **after** replica
manager, and use just a `Long` (with the semantics being changed as described
above)
3. Just use a Boolean and don't do the logging.
----------------------------------------------------------------
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:
us...@infra.apache.org
