[
https://issues.apache.org/jira/browse/KAFKA-339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397097#comment-13397097
]
Joel Koshy commented on KAFKA-339:
----------------------------------
Thanks for the patch. Some comments:
AbstractFetcher:
- currentOffset should never be empty, so we can get rid of the if.
- hasPartition, partitionCount need to synchronize fetchMap
- "shutting down" should probably be removed from the string on line 106
- newOffset can be computed from the messageSet - so the processPartitionData
implementation does not need to return the log end offset (and likewise when we
use this in the high-level consumer). It's probably safer to prevent
processPartitionData from overriding the new offset, and I don't see any
benefit in allowing it to do so.
AbstractFetcherManager:
- addFetcher:
- can rename to maybeAddFetcher
- also, maybe we should move the info log on line 38 to the None case and
print the fetcher id; and add another log for the other case saying there's
already a fetcher.
- What is the purpose of having a fetcher ID vs simply topic-partition?
- Should synchronize fetcherRunnableMap in shutdown with mapLock
ReplicaManager:
- Maybe some corner case that I'm missing, but makeFollower already passes in
the new leaderBrokerId so why do we need to re-read from ZooKeeper (line 173)?
ReplicaFetchTest:
- Ideally producer.close() should be before the waitUntilTrue
- The condition function uses & instead of &&.
- Also, instead of the hard-coded 60L I think it would be clearer and
sufficient to do something like:
val expectedOffset = brokers.head.getLogManager...logEndOffset
assertEquals(brokers.size, brokers.count( broker =>
broker.getLogManager...logEndOffset == expectedOffset ))
> using MultiFetch in the follower
> --------------------------------
>
> Key: KAFKA-339
> URL: https://issues.apache.org/jira/browse/KAFKA-339
> Project: Kafka
> Issue Type: Sub-task
> Components: core
> Affects Versions: 0.8
> Reporter: Jun Rao
> Assignee: Jun Rao
> Fix For: 0.8
>
> Attachments: kafka-339_v1.patch
>
> Original Estimate: 252h
> Remaining Estimate: 252h
>
> A broker could be following multiple topic/partitions from the broker.
> Instead of using 1 fetcher thread per topic/partition, it would be more
> efficient to use 1 fetcher thread that issues multi-fetch requests.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
