-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33548/#review82224
-----------------------------------------------------------

Ship it!


Thanks for the patch. +1. Just a few minor comments below. Also, could you 
trying using the following test code in 0.8.1 to see if v0 OffsetCommitRequest 
still works with 0.8.3 broker?


package kafka

import kafka.utils.Logging
import kafka.consumer.SimpleConsumer
import kafka.common.{OffsetMetadataAndError, TopicAndPartition}
import kafka.api.{OffsetFetchRequest, OffsetCommitRequest}


object OffsetCommitMain extends Logging {

def main(args: Array[String]): Unit = {
    val simpleConsumer = new SimpleConsumer("localhost", 9092, 1000000, 
64*1024, "test-client")

val topic = "topic"
// Commit an offset
val topicAndPartition = TopicAndPartition(topic, 0)
val commitRequest = OffsetCommitRequest("test-group", Map(topicAndPartition -> 
OffsetMetadataAndError(offset=42L)))
val commitResponse = simpleConsumer.commitOffsets(commitRequest)

System.out.println("OffsetCommitResponse: " + commitResponse.toString())

// Fetch it and verify
val fetchRequest = OffsetFetchRequest("test-group", Seq(topicAndPartition))
val fetchResponse = simpleConsumer.fetchOffsets(fetchRequest)

System.out.println("OffsetFetchResponse: " + fetchResponse.toString())


}
}


clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java
<https://reviews.apache.org/r/33548/#comment132925>

    It's probably clearer if we define OFFSET_FETCH_RESPONSE_V1.



core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala
<https://reviews.apache.org/r/33548/#comment132927>

    should not exist => Committed offset should not exist


- Jun Rao


On April 25, 2015, 7:59 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33548/
> -----------------------------------------------------------
> 
> (Updated April 25, 2015, 7:59 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2068
>     https://issues.apache.org/jira/browse/KAFKA-2068
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> 1. Remove timestamp in partition-info for offset-commit-request.v0
> 2. Handle offset-commit-request.v0 by writting to ZK.
> 3. Add offset-fetch-request.v1 with the same format as 
> offset-fetch-request.v0, which expects the same version of 
> offset-fetch-response (v0).
> 4. Handle offset-fetch-request.v0 by reading from ZK.
> 5. Minor changes in unit tests
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 
> d53fe45b9c5d5c873facd9696b1eacb67e812bca 
>   
> clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java
>  a0e19768ff400d74c87b592f6c25c666696727d2 
>   core/src/main/scala/kafka/api/OffsetCommitRequest.scala 
> cf8e6acc426aef6eb19d862bf6a108a5fc37907a 
>   core/src/main/scala/kafka/api/OffsetFetchRequest.scala 
> 67811a752a470bf9bdbc8c5419e8d6e20a006169 
>   core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 
> 139913f2a40a9afdf3baa7044af265afdebc1fda 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> b4004aa3a1456d337199aa1245fb0ae61f6add46 
>   core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 
> dbf9f48fac0150bc2f1e655030c67c21bd160735 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 
> 652208a70f66045b854549d93cbbc2b77c24b10b 
> 
> Diff: https://reviews.apache.org/r/33548/diff/
> 
> 
> Testing
> -------
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to