[
https://issues.apache.org/jira/browse/KAFKA-353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400201#comment-13400201
]
Jun Rao commented on KAFKA-353:
-------------------------------
Thanks for patch v1. Looks good overall. Some comments:
1. TopicData.partitionDatas: data is the plural form of datum. So datas feels
weird. How about partitionDataArray?
2. KafkaApis:
2.1 maybeUnblockDelayedRequests: put fetchRequestPurgatory.update in 1 line
2.2 handleFetchRequest:
For the following line:
if(fetchRequest.replicaId != -1) {
Instead of using -1 , could we create a constant like NoneFollowerReplicaId?
2.3 handleFetchRequest: When creating responses, we should fill in the elapsed
time instead of passing in -1. Note that the elapsed time is in nano-secs. So,
we probably should rename Response.elapsed to sth like elapsesNs. Ditto for
handleProduceRequest.
3. DelayedProduce:
3.1 isSatisfied(): need to handle the case when requiredAcks is default
(probably any value <0). This means whenever we get the ack from every replica
in the current ISR, the request is satisfied. This can be done by simply making
sure leader.HW >=
delayedProduce.partitionStatus(followerFetchPartition).localOffset.
3.2 Could we change localOffsets to sth like requiredOffsets?
3.3 respond(): need to compute elapse time in Response
3.4 We need to be aware that isSatisfied can be called concurrently on the same
DelayedProduce object. I am not sure if it's really necessary, but maybe we
should consider using AtomicBoolean for PartitionStatus.acksPending?
4. ProducerRequest: actTimeoutSecs should be actTimeoutMs.
5. ReplicaManager: I would vote for not checking ProducerRequestPurgatory when
there is a leadership change, since the logic is simpler. We just let the
producer requests timeout. If we do want to do the check here, we should share
the ProducerRequestPurgatory object used in KafkaApis.
6. SyncProducerConfigShared: We should probably make the default requiredAcks
to be -1 (i.e., wait for ack from each replica in ISR).
7. TopicMetadataTest: no need to change
8. 0.8 has moved. So need to rebase
> tie producer-side ack with high watermark and progress of replicas
> ------------------------------------------------------------------
>
> Key: KAFKA-353
> URL: https://issues.apache.org/jira/browse/KAFKA-353
> Project: Kafka
> Issue Type: Sub-task
> Affects Versions: 0.8
> Reporter: Jun Rao
> Assignee: Joel Koshy
> Attachments: kafka-353_v1.patch
>
>
--
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
