You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <wa...@gmail.com> on 2015/03/26 20:16:18 UTC
Re: Review Request 27391: Rebase patch and incorporate Joel's comments
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27391/
-----------------------------------------------------------
(Updated March 26, 2015, 7:16 p.m.)
Review request for kafka.
Summary (updated)
-----------------
Rebase patch and incorporate Joel's comments
Bugs: KAFKA-1634
https://issues.apache.org/jira/browse/KAFKA-1634
Repository: kafka
Description (updated)
-------
v1
last pass
minor TODO
Incorporate Joel's comments
incorporate Joel's comments round two
Joel's comment round three
Joel round four
dummy
Joel round four
unit tests
Joel and Jun's comments
incorporate Joel and Jun's comments
Joel's comments
Jun's comments
minor test fix
address comments
rebase and fixes
revert log4j
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java 436f9b2a843bc8c44d17403f5880b6736a5d56a8
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 101f382170ad6740b3f8ff2d27b93a64874a857f
clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 7672a3a0d674d101078651956d7122059e59e6d5
clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java 94e9d376235b3288836807d8e8d2547b3743aad5
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java 13237fd72da5448a3d596b882fef141f336f827d
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 1584a92447d276b6206d212b0b5487c352044154
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala cca815a128419e146feff53adaeddc901bb5de1f
core/src/main/scala/kafka/server/KafkaApis.scala 35af98f0bc1b6a50bd1d97a30147593f8c6a422d
core/src/main/scala/kafka/server/KafkaServer.scala dddef938fabae157ed8644536eb1a2f329fb42b7
core/src/main/scala/kafka/server/OffsetManager.scala d05e14d2018c0a0b5d22697313e9abde4363d65d
core/src/main/scala/kafka/server/ReplicaManager.scala c5274822c57bf3c1f9e4135c0bdcaa87ee50ce20
core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6
core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala e4d0435eb4213597c2fb9c3f2093c227de53a417
core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
Diff: https://reviews.apache.org/r/27391/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 27391: Rebase patch and incorporate Joel's comments
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27391/#review77970
-----------------------------------------------------------
Ship it!
Thank you for the updated patch. Just one minor comment below on suggested comments to add. Otherwise I think this looks good!
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/27391/#comment126341>
may be worth adding comments to describe these scenarios:
```
// - Commit timestamp is always set to now.
// - "Default" expiration timestamp is now + retention (and retention may be overridden if v2)
// - Expire timestamp is computed differently for v1 and v2.
// - If v1 and no explicit commit timestamp is provided we use default expiration timestamp.
// - If v1 and explicit commit timestamp is provided we calculate retention from that explicit commit timestamp
// - If v2 we use the default expiration timestamp
```
- Joel Koshy
On March 26, 2015, 7:28 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27391/
> -----------------------------------------------------------
>
> (Updated March 26, 2015, 7:28 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1634
> https://issues.apache.org/jira/browse/KAFKA-1634
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 1. Always override commit time to now, only change expire time accorinding to the commit time.
> 2. Override expire time upon loading offsets of version 1.
> 3. Change new consumer to use the new version 2.
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java 436f9b2a843bc8c44d17403f5880b6736a5d56a8
> clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 101f382170ad6740b3f8ff2d27b93a64874a857f
> clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 7672a3a0d674d101078651956d7122059e59e6d5
> clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java 94e9d376235b3288836807d8e8d2547b3743aad5
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java 13237fd72da5448a3d596b882fef141f336f827d
> 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 1584a92447d276b6206d212b0b5487c352044154
> core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala cca815a128419e146feff53adaeddc901bb5de1f
> core/src/main/scala/kafka/server/KafkaApis.scala 35af98f0bc1b6a50bd1d97a30147593f8c6a422d
> core/src/main/scala/kafka/server/KafkaConfig.scala 46d21c73f1feb3410751899380b35da0c37c975c
> core/src/main/scala/kafka/server/KafkaServer.scala dddef938fabae157ed8644536eb1a2f329fb42b7
> core/src/main/scala/kafka/server/OffsetManager.scala d05e14d2018c0a0b5d22697313e9abde4363d65d
> core/src/main/scala/kafka/server/ReplicaManager.scala c5274822c57bf3c1f9e4135c0bdcaa87ee50ce20
> core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6
> core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala e4d0435eb4213597c2fb9c3f2093c227de53a417
> core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
>
> Diff: https://reviews.apache.org/r/27391/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Guozhang Wang
>
>
Re: Review Request 27391: Rebase patch and incorporate Joel's comments
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27391/
-----------------------------------------------------------
(Updated March 26, 2015, 7:28 p.m.)
Review request for kafka.
Bugs: KAFKA-1634
https://issues.apache.org/jira/browse/KAFKA-1634
Repository: kafka
Description (updated)
-------
1. Always override commit time to now, only change expire time accorinding to the commit time.
2. Override expire time upon loading offsets of version 1.
3. Change new consumer to use the new version 2.
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java 436f9b2a843bc8c44d17403f5880b6736a5d56a8
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 101f382170ad6740b3f8ff2d27b93a64874a857f
clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 7672a3a0d674d101078651956d7122059e59e6d5
clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java 94e9d376235b3288836807d8e8d2547b3743aad5
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java 13237fd72da5448a3d596b882fef141f336f827d
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 1584a92447d276b6206d212b0b5487c352044154
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala cca815a128419e146feff53adaeddc901bb5de1f
core/src/main/scala/kafka/server/KafkaApis.scala 35af98f0bc1b6a50bd1d97a30147593f8c6a422d
core/src/main/scala/kafka/server/KafkaConfig.scala 46d21c73f1feb3410751899380b35da0c37c975c
core/src/main/scala/kafka/server/KafkaServer.scala dddef938fabae157ed8644536eb1a2f329fb42b7
core/src/main/scala/kafka/server/OffsetManager.scala d05e14d2018c0a0b5d22697313e9abde4363d65d
core/src/main/scala/kafka/server/ReplicaManager.scala c5274822c57bf3c1f9e4135c0bdcaa87ee50ce20
core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6
core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala e4d0435eb4213597c2fb9c3f2093c227de53a417
core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
Diff: https://reviews.apache.org/r/27391/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 27391: Rebase patch and incorporate Joel's comments
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27391/
-----------------------------------------------------------
(Updated March 26, 2015, 7:27 p.m.)
Review request for kafka.
Bugs: KAFKA-1634
https://issues.apache.org/jira/browse/KAFKA-1634
Repository: kafka
Description (updated)
-------
TBD
Diffs (updated)
-----
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java 436f9b2a843bc8c44d17403f5880b6736a5d56a8
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 101f382170ad6740b3f8ff2d27b93a64874a857f
clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 7672a3a0d674d101078651956d7122059e59e6d5
clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java 94e9d376235b3288836807d8e8d2547b3743aad5
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java 13237fd72da5448a3d596b882fef141f336f827d
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 1584a92447d276b6206d212b0b5487c352044154
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala cca815a128419e146feff53adaeddc901bb5de1f
core/src/main/scala/kafka/server/KafkaApis.scala 35af98f0bc1b6a50bd1d97a30147593f8c6a422d
core/src/main/scala/kafka/server/KafkaConfig.scala 46d21c73f1feb3410751899380b35da0c37c975c
core/src/main/scala/kafka/server/KafkaServer.scala dddef938fabae157ed8644536eb1a2f329fb42b7
core/src/main/scala/kafka/server/OffsetManager.scala d05e14d2018c0a0b5d22697313e9abde4363d65d
core/src/main/scala/kafka/server/ReplicaManager.scala c5274822c57bf3c1f9e4135c0bdcaa87ee50ce20
core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6
core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala e4d0435eb4213597c2fb9c3f2093c227de53a417
core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
Diff: https://reviews.apache.org/r/27391/diff/
Testing
-------
Thanks,
Guozhang Wang
Re: Review Request 27391: Rebase patch and incorporate Joel's comments
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27391/
-----------------------------------------------------------
(Updated March 26, 2015, 7:17 p.m.)
Review request for kafka.
Bugs: KAFKA-1634
https://issues.apache.org/jira/browse/KAFKA-1634
Repository: kafka
Description (updated)
-------
1. Always override commit time to now, only change expire time accorinding to the commit time.
2. Override expire time upon loading offsets of version 1.
3. Change new consumer to use the new version 2.
Diffs
-----
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java 436f9b2a843bc8c44d17403f5880b6736a5d56a8
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 101f382170ad6740b3f8ff2d27b93a64874a857f
clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 7672a3a0d674d101078651956d7122059e59e6d5
clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java 94e9d376235b3288836807d8e8d2547b3743aad5
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java 13237fd72da5448a3d596b882fef141f336f827d
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 1584a92447d276b6206d212b0b5487c352044154
core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala cca815a128419e146feff53adaeddc901bb5de1f
core/src/main/scala/kafka/server/KafkaApis.scala 35af98f0bc1b6a50bd1d97a30147593f8c6a422d
core/src/main/scala/kafka/server/KafkaServer.scala dddef938fabae157ed8644536eb1a2f329fb42b7
core/src/main/scala/kafka/server/OffsetManager.scala d05e14d2018c0a0b5d22697313e9abde4363d65d
core/src/main/scala/kafka/server/ReplicaManager.scala c5274822c57bf3c1f9e4135c0bdcaa87ee50ce20
core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6
core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala e4d0435eb4213597c2fb9c3f2093c227de53a417
core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala b46daa436231d5aa5c1e2992fd5c2d9a73a30c80
Diff: https://reviews.apache.org/r/27391/diff/
Testing
-------
Thanks,
Guozhang Wang