[ 
https://issues.apache.org/jira/browse/KAFKA-391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448915#comment-13448915
 ] 

Jun Rao commented on KAFKA-391:
-------------------------------

Thanks for the patch. Some comments:

1. We probably shouldn't use scala sortedMap in kafka.javaapi.ProducerRequest. 
On that thought, why can't ProducerRequest take a regular map in the 
constructor? If we want some ordering on the serialized data, we can sort the 
map before serialization. SortedMap seems to reveal an implementation detail 
that clients don't (and shouldn't) really care. Ditto for ProducerResponse.

2. To be consistent, should we change FetchResponse (and maybe FetchRequest) to 
use map, instead of array too?

3. ProducerRequest.writeTo() can use foreach like the following:
   groupedData.foreach{ case(topic, TopciAndPartitionData) => ... }

                
> Producer request and response classes should use maps
> -----------------------------------------------------
>
>                 Key: KAFKA-391
>                 URL: https://issues.apache.org/jira/browse/KAFKA-391
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Joel Koshy
>            Assignee: Joel Koshy
>              Labels: optimization
>             Fix For: 0.8
>
>         Attachments: KAFKA-391-draft-r1374069.patch
>
>
> Producer request contains an array of TopicData which contains arrays of 
> PartitionData.
> Producer response contains two arrays of error codes and offsets - the 
> ordering in these arrays correspond to the flattened ordering of the request 
> arrays.
> It would be better to switch to maps in the request and response as this 
> would make the code clearer and more efficient (right now, linear scans are 
> used in handling producer acks).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to