> On Nov. 17, 2014, 10:31 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > line 151 > > <https://reviews.apache.org/r/27391/diff/3/?file=755182#file755182line151> > > > > Made a follow-up comment in the earlier RB. But pasting here as well: > > > > Agree that it is still "common" in the object but it is completely > > removed from the wire protocol - i.e., OFFSET_COMMIT_REQUEST_PARTITION_V1 > > which is in the V2 OffsetCommitRequest does not have a timestamp. > > > > This method should probably be read as "initCommonFieldsInStruct" - > > i.e., effectively the wire protocol. That said, I'm loathe to add another > > init method which reads initCommonFieldsInV0AndV1. So I think rather than > > checking fetchPartitionData.timestamp it would be better to explicitly > > check the (already set) request version in the struct. If v0 or v1 then set > > the timestamp key name.
The version number info is not in the struct, so we cannot get its value unless we are going to add that from the request header into the constructor. But we can check if the struct has that field or not. Changed to this accordingly. - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review61761 ----------------------------------------------------------- On Nov. 8, 2014, 12:54 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27391/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2014, 12:54 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1634 > https://issues.apache.org/jira/browse/KAFKA-1634 > > > Repository: kafka > > > Description > ------- > > The timestamp field of OffsetAndMetadata is preserved since we need to be > backward compatible with older versions > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 7517b879866fc5dad5f8d8ad30636da8bbe7784a > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java > 3ee5cbad55ce836fd04bb954dcf6ef2f9bc3288f > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > df37fc6d8f0db0b8192a948426af603be3444da4 > core/src/main/scala/kafka/api/OffsetCommitRequest.scala > 050615c72efe7dbaa4634f53943bd73273d20ffb > core/src/main/scala/kafka/api/OffsetFetchRequest.scala > c7604b9cdeb8f46507f04ed7d35fcb3d5f150713 > core/src/main/scala/kafka/common/OffsetMetadataAndError.scala > 4cabffeacea09a49913505db19a96a55d58c0909 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > fbc680fde21b02f11285a4f4b442987356abd17b > core/src/main/scala/kafka/server/KafkaApis.scala > 968b0c4f809ea9f9c3f322aef9db105f9d2e0e23 > core/src/main/scala/kafka/server/KafkaServer.scala > 4de812374e8fb1fed834d2be3f9655f55b511a74 > core/src/main/scala/kafka/server/OffsetManager.scala > 2957bc435102bc4004d8f100dbcdd56287c8ffae > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > cd16ced5465d098be7a60498326b2a98c248f343 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 8c5364fa97da1be09973c176d1baeb339455d319 > > Diff: https://reviews.apache.org/r/27391/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
