> On Jan. 20, 2015, 4:35 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 147 > > <https://reviews.apache.org/r/27391/diff/8/?file=822017#file822017line147> > > > > I just realized that if we have a v0 or v1 request then we use the > > offset manager default retention which is one day. > > > > However, if it is v2 and the user does not override it in the offset > > commit request, then the retention defaults to Long.MaxValue. I think that > > default makes sense for OffsetCommitRequest. However, I think the broker > > needs to protect itself and have an upper threshold for retention. i.e., > > maybe we should have a maxRetentionMs config in the broker. > > > > What do you think?
Agreed, I change the behavior to be "use the default value if it is < v2 or if the retention period is default value (meaning user did not specify it)". - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review68729 ----------------------------------------------------------- On Jan. 14, 2015, 11:50 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27391/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2015, 11:50 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1634 > https://issues.apache.org/jira/browse/KAFKA-1634 > > > Repository: kafka > > > Description > ------- > > Incorporated Joel and Jun's comments > > > 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 > 191a8677444e53b043e9ad6e94c5a9191c32599e > core/src/main/scala/kafka/server/KafkaApis.scala > c011a1b79bd6c4e832fe7d097daacb0d647d1cd4 > core/src/main/scala/kafka/server/KafkaServer.scala > a069eb9272c92ef62387304b60de1fe473d7ff49 > core/src/main/scala/kafka/server/OffsetManager.scala > 3c79428962604800983415f6f705e04f52acb8fb > core/src/main/scala/kafka/server/ReplicaManager.scala > e58fbb922e93b0c31dff04f187fcadb4ece986d7 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > cd16ced5465d098be7a60498326b2a98c248f343 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 4a3a5b264a021e55c39f4d7424ce04ee591503ef > core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala > ba1e48e4300c9fb32e36e7266cb05294f2a481e5 > > Diff: https://reviews.apache.org/r/27391/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >