[
https://issues.apache.org/jira/browse/KAFKA-8400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16847993#comment-16847993
]
ASF GitHub Bot commented on KAFKA-8400:
---------------------------------------
hachikuji commented on pull request #6814: KAFKA-8400; Do not update follower
replica state if the log read failed
URL: https://github.com/apache/kafka/pull/6814
This patch checks for errors handling a fetch request before updating
follower state. Previously we were unsafely passing the failed `LogReadResult`
with most fields set to -1 into `Replica` to update follower state.
Additionally, this patch attempts to improve the test coverage for ISR
shrinking and expansion logic in `Partition`.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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
> Do not update follower replica state if the log read failed
> -----------------------------------------------------------
>
> Key: KAFKA-8400
> URL: https://issues.apache.org/jira/browse/KAFKA-8400
> Project: Kafka
> Issue Type: Improvement
> Reporter: Jason Gustafson
> Assignee: Jason Gustafson
> Priority: Major
>
> In {{ReplicaManager.fetchMessages}}, we have the following logic to read from
> the log and update follower state:
> {code}
> val result = readFromLocalLog(
> replicaId = replicaId,
> fetchOnlyFromLeader = fetchOnlyFromLeader,
> fetchIsolation = fetchIsolation,
> fetchMaxBytes = fetchMaxBytes,
> hardMaxBytesLimit = hardMaxBytesLimit,
> readPartitionInfo = fetchInfos,
> quota = quota)
> if (isFromFollower) updateFollowerLogReadResults(replicaId, result)
> else result
> {code}
> The call to {{readFromLocalLog}} could fail for many reasons, in which case
> we return a LogReadResult with an error set and all fields set to -1. The
> problem is that we do not check for the error when updating the follower
> state. As far as I can tell, this does not cause any correctness issues, but
> we're just asking for trouble. It would be better to check the error before
> proceeding to `Partition.updateReplicaLogReadResult`.
> Perhaps even better would be to have {{readFromLocalLog}} return something
> like {{Either[LogReadResult, Errors]}} so that we are forced to handle the
> error.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
