kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jun Rao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KAFKA-657) Add an API to commit offsets
Date Wed, 02 Jan 2013 23:24:12 GMT

    [ 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

Mime
View raw message