[
https://issues.apache.org/jira/browse/KAFKA-657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13542540#comment-13542540
]
Jun Rao commented on KAFKA-657:
-------------------------------
Thanks for patch v7. Some more comments:
70. OffsetFetchResponse: requestInfo can be defined as Map[TopicAndPartition,
(Long, Short)] without referencing Tuple2 directly.
71. KafkaApis.handle():
case (topicAndPartition) => (topicAndPartition, Tuple2(-1L,
ErrorMapping.codeFor(e.getClass.asInstanceOf[Class[Throwable]]))) can be
case (topicAndPartition) => (topicAndPartition, (-1L,
ErrorMapping.codeFor(e.getClass.asInstanceOf[Class[Throwable]])))
Also, instead of using -1, can we define a constant like InvalidOffset? Use
that constant in handleOffsetFetchRequest() too.
72. javaapi.OffsetCommitResponse and javaapi.OffsetFetchResponse: Remove
unused import ByteBuffer.
73. javaapi.OffsetFetchResponse: Tuple2 is a scala thing. It would be weird to
return that directly to the java caller. One way is to change the type of
requestInfo in the scala version of OffsetFetchResponse to
Map[TopicAndPartition, OffsetAndError]. This also follows our convention of
limiting the usage of Tuple.
74. java version of OffsetCommit{Request/Response} and
OffsetFetch{Request/Response}: We don't really need the methods sizeInBytes()
and writeTo(). They are only used for serialization and we don't serialize the
java version of the request/response. This issue seems to already exist in the
java version of OffsetRequest and FetchRequest. Could you remove these two
methods there too?
75. OffsetCommitTest:
75.1 Remove unused imports.
75.2 Wrong spelling non-existant.
75.3 When handling OffsetCommitRequests, we do exactly the same thing whether a
topic/partition exists in KafkaApis. So, it doesn't seem that we need to test
unknown topic/partition separately.
> Add an API to commit offsets
> ----------------------------
>
> Key: KAFKA-657
> URL: https://issues.apache.org/jira/browse/KAFKA-657
> Project: Kafka
> Issue Type: New Feature
> Reporter: Jay Kreps
> Labels: project
> Attachments: KAFKA-657v1.patch, KAFKA-657v2.patch, KAFKA-657v3.patch,
> KAFKA-657v4.patch, KAFKA-657v5.patch, KAFKA-657v6.patch, KAFKA-657v7.patch
>
>
> Currently the consumer directly writes their offsets to zookeeper. Two
> problems with this: (1) This is a poor use of zookeeper, and we need to
> replace it with a more scalable offset store, and (2) it makes it hard to
> carry over to clients in other languages. A first step towards accomplishing
> that is to add a proper Kafka API for committing offsets. The initial version
> of this would just write to zookeeper as we do today, but in the future we
> would then have the option of changing this.
> This api likely needs to take a sequence of
> consumer-group/topic/partition/offset entries and commit them all.
> It would be good to do a wiki design on how this would work and consensus on
> that first.
--
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
