[
https://issues.apache.org/jira/browse/KAFKA-6795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16449275#comment-16449275
]
Anna Povzner edited comment on KAFKA-6795 at 4/24/18 6:03 AM:
--------------------------------------------------------------
[~lindong] Before reviewing unit tests, could you please confirm the behavior
of ReplicaAlterLogDirsThread.buildLeaderEpochRequest()? I think, the right
behavior is to build offset for leader epoch requests based on *future*
replica's epochCache and latestEpoch(). Since *future* replica is the fetching
replica, so it should request an offset based on its own latestEpoch() vs.
using latestEpoch() of topic partition replica. And then
fetchEpochsFromLeader() correctly fetches endOffsetFor() based on latest epoch
of *future* replica. Otherwise, it is basically fetches the offset based on
"leader" epoch.
The PR includes the proposed fix.
was (Author: apovzner):
[~lindong] Before reviewing unit tests, could you please confirm the behavior
of ReplicaAlterLogDirsThread.buildLeaderEpochRequest()? I think, the right
behavior is to build offset for leader epoch requests based on *future*
replica's epochCache and latestEpoch(). Since *future* replica is the fetching
replica, so it should request an offset based on its own latestEpoch() vs.
using latestEpoch() of topic partition replica. And then
fetchEpochsFromLeader() correctly fetches endOffsetFor() based on latest epoch
of *future* replica. Otherwise, it is basically fetches the offset based on
"leader" epoch.
It was actually hard to notice, only after I realized that I was mocking
latestEpoch() of the source replica vs. destination. The PR currently has tests
which follow the current code (meaning they pass), but if you agree regarding
the implementation of ReplicaAlterLogDirsThread.buildLeaderEpochRequest(), then
I will need to fix the tests as well.
I think the right implementation is to replace:
{{private def epochCacheOpt(tp: TopicPartition): Option[LeaderEpochCache] =
replicaMgr.getReplica(tp).map(_.epochs.get)}}
with
{{private def epochCacheOpt(tp: TopicPartition): Option[LeaderEpochCache] =
replicaMgr.getReplica(tp, Request.FutureLocalReplicaId).map(_.epochs.get)}}
> Add unit test for ReplicaAlterLogDirsThread
> -------------------------------------------
>
> Key: KAFKA-6795
> URL: https://issues.apache.org/jira/browse/KAFKA-6795
> Project: Kafka
> Issue Type: Improvement
> Reporter: Anna Povzner
> Assignee: Anna Povzner
> Priority: Major
>
> ReplicaAlterLogDirsThread was added as part of KIP-113 implementation, but
> there is no unit test.
> [~lindong] I assigned this to myself, since ideally I wanted to add unit
> tests for KAFKA-6361 related changes (KIP-279), but feel free to re-assign.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
