> On Jan. 8, 2015, 2:24 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > lines 79-85 > > <https://reviews.apache.org/r/27391/diff/1/?file=743505#file743505line79> > > > > Perhaps these code can just be changed to > > > > this(groupId, DEFAULT_GENERATION_ID, DEFAULT_CONSUMER_ID, offsetData);
This cannot be forwarded as in super() call we need to specify the version id. > On Jan. 8, 2015, 2:24 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > lines 97-106 > > <https://reviews.apache.org/r/27391/diff/1/?file=743505#file743505line97> > > > > Same here. These code can just be replaced by forwarding the request to > > the next constructor. Ditto above. > On Jan. 8, 2015, 2:24 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, lines 524-525 > > <https://reviews.apache.org/r/27391/diff/7/?file=779891#file779891line524> > > > > Shouldn't we just set the expiration time field to expirationTimestamp, > > instead of taking it from offsetAndMetadata? For v0/1, we should just take the value of the offsetAndMetadata.timestamp, for v2 we will take the value of expirationTimestamp. This has been changed in the latest patch where offsetAndMetadata.timestamp is updated accordingly before calling offsetCommitValue(). - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review67114 ----------------------------------------------------------- On Dec. 2, 2014, 2:03 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27391/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 2:03 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1634 > https://issues.apache.org/jira/browse/KAFKA-1634 > > > Repository: kafka > > > Description > ------- > > Add another api in offset manager to return the struct, and the cache layer > will only read its expiration timestamp while the offset formatter will read > the struct as a whole > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 7517b879866fc5dad5f8d8ad30636da8bbe7784a > clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java > 121e880a941fcd3e6392859edba11a94236494cc > > 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 > da29a8cb461099eb675161db2f11a9937424a5c6 > core/src/main/scala/kafka/server/KafkaApis.scala > 2a1c0326b6e6966d8b8254bd6a1cb83ad98a3b80 > core/src/main/scala/kafka/server/KafkaServer.scala > 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe > core/src/main/scala/kafka/server/OffsetManager.scala > 3c79428962604800983415f6f705e04f52acb8fb > 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 > >
