Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kafka.apache.org
Date: Wed, 2 Jan 2013 23:24:12 +0000 (UTC)
From: "Jun Rao (JIRA)" <jira@apache.org>
To: dev@kafka.apache.org
Message-ID: <JIRA.12619154.1354812529928.71654.1357169052556@arcas>
In-Reply-To: <JIRA.12619154.1354812529928@arcas>
References: <JIRA.12619154.1354812529928@arcas>
Subject: [jira] [Commented] (KAFKA-657) Add an API to commit offsets
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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