Re: Review Request 27391: Fix KAFKA-1634

> On Feb. 4, 2015, 2:15 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/OffsetCommitRequest.scala, line 48 > > > > > > I our convention is to include the if in the previous line. > > Guozhang Wang wrote: >

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Feb. 6, 2015, 7:01 p.m.) Review request for kafka. Bugs: KAFKA-1634

Re: Review Request 27391: Fix KAFKA-1634

> On Feb. 4, 2015, 2:15 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/OffsetCommitRequest.scala, line 48 > > > > > > I our convention is to include the if in the previous line. I checked the code base and

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review70836 --- clients/src/main/java/org/apache/kafka/common/protocol/Protocol.jav

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Jan. 24, 2015, 12:06 a.m.) Review request for kafka. Bugs: KAFKA-163

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Jan. 23, 2015, 2:47 a.m.) Review request for kafka. Bugs: KAFKA-1634

Re: Review Request 27391: Fix KAFKA-1634

> On Jan. 22, 2015, 1:56 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, lines 145-166 > > > > > > I am not sure that we should change the timestamp for offsets produced > > in V0 and V1. Th

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review69106 --- Thanks for the patch. A few more comments. clients/src/main/java/o

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Jan. 22, 2015, 12:43 a.m.) Review request for kafka. Bugs: KAFKA-163

Re: Review Request 27391: Fix KAFKA-1634

> On Jan. 20, 2015, 4:35 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 147 > > > > > > I just realized that if we have a v0 or v1 request then we use the > > offset manager default

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review68729 --- core/src/main/scala/kafka/server/KafkaApis.scala

Re: Review Request 27391: Fix KAFKA-1634

--- 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-163

Re: Review Request 27391: Fix KAFKA-1634

> 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 > > > > > > Perhaps these code can just be changed to > > > >

Re: Review Request 27391: Fix KAFKA-1634

> On Dec. 18, 2014, 8:42 a.m., Joel Koshy wrote: > > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala, line 224 > > > > > > If the offset in fact did expire, the assertion itself won't fail - > > i.e., you

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review67114 --- clients/src/main/java/org/apache/kafka/common/requests/OffsetCommit

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review65462 --- clients/src/main/java/org/apache/kafka/common/requests/OffsetCommit

Re: Review Request 27391: Fix KAFKA-1634

--- 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

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Dec. 1, 2014, 7:44 p.m.) Review request for kafka. Bugs: KAFKA-1634

Re: Review Request 27391: Fix KAFKA-1634

> On Nov. 21, 2014, 2:41 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/common/OffsetMetadataAndError.scala, line 17 > > > > > > I thought we would be going with separate format for on-disk storage? > > > >

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Nov. 21, 2014, 10 p.m.) Review request for kafka. Bugs: KAFKA-1634

Re: Review Request 27391: Fix KAFKA-1634

> On Nov. 21, 2014, 2:41 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 500 > > > > > > This and the above use give compilation warnings. I think it is > > reasonable to "copy"

Re: Review Request 27391: Fix KAFKA-1634

> On Nov. 21, 2014, 2:41 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 147 > > > > > > I think what you meant earlier was with < v2 you could have different > > timestamps for each

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review62553 --- core/src/main/scala/kafka/api/OffsetCommitRequest.scala

Re: Review Request 27391: Fix KAFKA-1634

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Nov. 18, 2014, 1:42 a.m.) Review request for kafka. Summary (updated

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 7, 2014, 3 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 498 > > > > > > Actually, since we always set the retention period (for v0, v1) in > > KafkaApis do we need t

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 17, 2014, 10:31 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > line 151 > > > > > > Made a follow-up comment in the earlier RB. But pasting h

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 7, 2014, 3 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > line 152 > > > > > > This confused me a bit, and I think it is because initCommonFi

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review61761 --- clients/src/main/java/org/apache/kafka/common/requests/OffsetCommit

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- 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

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 7, 2014, 3 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 498 > > > > > > Actually, since we always set the retention period (for v0, v1) in > > KafkaApis do we need t

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 7, 2014, 3 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/common/OffsetMetadataAndError.scala, line 22 > > > > > > Can we mark this @Deprecated as well? > > > > We should probably make the prim

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

> On Nov. 7, 2014, 3 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java, > > line 152 > > > > > > This confused me a bit, and I think it is because initCommonFi

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review60285 --- clients/src/main/java/org/apache/kafka/common/requests/OffsetCommit

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- (Updated Nov. 6, 2014, 11:35 p.m.) Review request for kafka. Bugs: KAFKA-1634

Re: Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/#review59839 --- clients/src/main/java/org/apache/kafka/common/requests/OffsetCommit

Review Request 27391: Fix KAFKA-1634: Add the global retentionTime (in milis) and disable the per-offset timestamp

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27391/ --- Review request for kafka. Bugs: KAFKA-1634 https://issues.apache.org/jira/b