You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Andrii Biletskyi <an...@stealth.ly> on 2015/01/21 18:49:10 UTC

Review Request 30126: Patch for KAFKA-1845

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1845
    https://issues.apache.org/jira/browse/KAFKA-1845


Repository: kafka


Description
-------

KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored


Diffs
-----

  clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
  core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
  core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
  core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
  core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
  core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
  core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
  core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
  core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
  core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
  core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
  core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
  core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
  core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
  core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
  core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
  core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
  core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
  core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
  core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
  core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
  core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
  core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
  core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 

Diff: https://reviews.apache.org/r/30126/diff/


Testing
-------


Thanks,

Andrii Biletskyi


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Joe Stein <cr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70747
-----------------------------------------------------------



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116124>

    LOW



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116125>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116126>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116127>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116128>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116147>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116131>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116134>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116132>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116133>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116135>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116136>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116137>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116138>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116139>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116140>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116141>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116142>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116143>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116144>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116145>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116146>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116148>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116149>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116150>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116152>

    MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116153>

    MEDIUM


- Joe Stein


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Andrii Biletskyi <an...@stealth.ly>.

> On Feb. 3, 2015, 2:06 p.m., Jeff Holoman wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 834
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line834>
> >
> >     Is this is the right way to do this rather than having these in Validators?

Some of the broker settings validations are quite intricate - they involve other settings to be compared (e.g. replicaFetchWaitMaxMs <= replicaSocketTimeoutMs), which we can't do inside ConfigDef.define.


- Andrii


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70748
-----------------------------------------------------------


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Jeff Holoman <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70748
-----------------------------------------------------------



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment116129>

    Is this is the right way to do this rather than having these in Validators?


- Jeff Holoman


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Andrii Biletskyi <an...@stealth.ly>.

> On Jan. 22, 2015, 7:57 p.m., Gwen Shapira wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 743-758
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line743>
> >
> >     This looks fairly generic. Shouldn't it be somewhere where it can be reused by other Config classes? Perhaps in ConfigDef or something similar?

I've fixed the function slightly - "valid" predicate is needless here (it was ported from VerifiableProperties), so it's just really Try[] arround already utility function Utils.parseCsvMap(?str) . I doubt it worth moving simple try-catch to separate function now... Hope this resolves.


- Andrii


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69224
-----------------------------------------------------------


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69224
-----------------------------------------------------------


Looks great. Just one minor comment.


core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment113845>

    This looks fairly generic. Shouldn't it be somewhere where it can be reused by other Config classes? Perhaps in ConfigDef or something similar?


- Gwen Shapira


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Andrii Biletskyi <an...@stealth.ly>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/
-----------------------------------------------------------

(Updated March 4, 2015, 11:12 p.m.)


Review request for kafka.


Bugs: KAFKA-1845
    https://issues.apache.org/jira/browse/KAFKA-1845


Repository: kafka


Description (updated)
-------

KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored


KAFKA-1845 - code review fixes, merge conflicts after rebase


KAFKA-1845 - rebase to trunk


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 8523333a9b39a2f9cd71d176943be86531a43ede 
  core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
  core/src/main/scala/kafka/controller/KafkaController.scala e9b4dc62df3f139de8d4dc688e48ccc0a5513123 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
  core/src/main/scala/kafka/server/KafkaConfig.scala 14bf3216bae030331bdf76b3266ed0e73526c3de 
  core/src/main/scala/kafka/server/KafkaServer.scala 8e3def9e9edaf49c0443a2e08cf37277d0f25306 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
  core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala 5650b4a7b950b48af3e272947bfb5e271c4238c9 
  core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala d34ee3a40dcc8475e183435ad6842cd3d0a13ade 
  core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 8154a4210dc8dcceb26549c98bbbc4a1282a1c67 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
  core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala d530338728be41282925b3a62030d1f316b4f9c5 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala c8f336aa034ab5702c7644a669fd32c746512d29 
  core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
  core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
  core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
  core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala aeb7a19acaefabcc161c2ee6144a56d9a8999a81 
  core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
  core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
  core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
  core/src/test/scala/unit/kafka/log/LogTest.scala 1a4be70a21fe799d4d71dd13e84968e40fb8ad92 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
  core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 111e4a26c1efb6f7c151ca9217dbe107c27ab617 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
  core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
  core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
  core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
  core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
  core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala a37a74dc89edca0c03150db17ee9dff1dac15606 
  core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala d1ed5c2c506898c2978860a0ec6f6767d42370a0 
  core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 82fa4cff45043e74825c12f3c8494fa8fc903334 
  core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 93af7dfcec7a6b1c1adfbe278d30d8406a84b60d 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala fd8f32c4158d44cb13ec7f27de11ee60aca49fd9 
  core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 305498adf416a46d59e5c266f3417a38f529be2d 

Diff: https://reviews.apache.org/r/30126/diff/


Testing
-------


Thanks,

Andrii Biletskyi


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Andrii Biletskyi <an...@stealth.ly>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/
-----------------------------------------------------------

(Updated Feb. 8, 2015, 3:05 p.m.)


Review request for kafka.


Bugs: KAFKA-1845
    https://issues.apache.org/jira/browse/KAFKA-1845


Repository: kafka


Description (updated)
-------

KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig


KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored


KAFKA-1845 - code review fixes, merge conflicts after rebase


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 8523333a9b39a2f9cd71d176943be86531a43ede 
  core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
  core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
  core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
  core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
  core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala 5650b4a7b950b48af3e272947bfb5e271c4238c9 
  core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
  core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
  core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
  core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
  core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
  core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
  core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala aeb7a19acaefabcc161c2ee6144a56d9a8999a81 
  core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
  core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
  core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
  core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
  core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
  core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
  core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
  core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
  core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
  core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
  core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
  core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
  core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
  core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
  core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
  core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
  core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
  core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
  core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
  core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 305498adf416a46d59e5c266f3417a38f529be2d 

Diff: https://reviews.apache.org/r/30126/diff/


Testing
-------


Thanks,

Andrii Biletskyi


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Andrii Biletskyi <an...@stealth.ly>.

> On Jan. 21, 2015, 10:55 p.m., Eric Olander wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 130
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line130>
> >
> >     It seems that by convention there is a ...Prop and a ...Doc constant, but nothing enforces that.  Maybe have 
> >     val ZKConnect = ("zookeeper.connect", "Zookeeper host string") 
> >     so it is more apparent that these two values are needed and related.  A utility class would be better than using a Tuple2, but that's the general idea.

you mean smth like 
```
case class Key(prop: String, doc: String)
```
?
I can even do 
```
val (ZkConnectProp, ZkConnectDoc) = ("zookeeper.connect", "Zookeeper host string")
```
I don't mind, it's just ProducerConfig and LogConfig then won't follow this approach and again we'll end up with not uniform config implementations...
Let's wait if others support this since it's a big change and I'd rather do it once :)


> On Jan. 21, 2015, 10:55 p.m., Eric Olander wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 475
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line475>
> >
> >     Maybe some helper functions could help with this code:
> >     
> >     def stringProp(prop: String) = parsed.get(prop).asInstanceOf[String]
> >     
> >     then:
> >     zkConnect = stringProp(ZkConnectProp)
> 
> Gwen Shapira wrote:
>     I like this suggestion. I'd add it to ConfigDef or something similar, so all Config classes can enjoy this.
>     This can be a follow-up patch, since we are already using the parsed.get.asInstanceOf pattern everywhere, so the fix is not related to this patch specifically.

Yes, good point!
Just one thing: if we add it to ConfigDef java class (which I like too), we won't be able to use implicits so the utility function will be less gracefull:
```
String  stringProp(parsed: Map<String, Object>, prop: String)
``` 
and sample usage:
```
zkConnect = stringProp(parsed, ZkConnectProp)
```


- Andrii


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69063
-----------------------------------------------------------


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Gwen Shapira <gs...@cloudera.com>.

> On Jan. 21, 2015, 10:55 p.m., Eric Olander wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 475
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line475>
> >
> >     Maybe some helper functions could help with this code:
> >     
> >     def stringProp(prop: String) = parsed.get(prop).asInstanceOf[String]
> >     
> >     then:
> >     zkConnect = stringProp(ZkConnectProp)

I like this suggestion. I'd add it to ConfigDef or something similar, so all Config classes can enjoy this.
This can be a follow-up patch, since we are already using the parsed.get.asInstanceOf pattern everywhere, so the fix is not related to this patch specifically.


- Gwen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69063
-----------------------------------------------------------


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>


Re: Review Request 30126: Patch for KAFKA-1845

Posted by Eric Olander <ol...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69063
-----------------------------------------------------------



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment113635>

    It seems that by convention there is a ...Prop and a ...Doc constant, but nothing enforces that.  Maybe have 
    val ZKConnect = ("zookeeper.connect", "Zookeeper host string") 
    so it is more apparent that these two values are needed and related.  A utility class would be better than using a Tuple2, but that's the general idea.



core/src/main/scala/kafka/server/KafkaConfig.scala
<https://reviews.apache.org/r/30126/#comment113634>

    Maybe some helper functions could help with this code:
    
    def stringProp(prop: String) = parsed.get(prop).asInstanceOf[String]
    
    then:
    zkConnect = stringProp(ZkConnectProp)


- Eric Olander


On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 5:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1845
>     https://issues.apache.org/jira/browse/KAFKA-1845
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on instantiating KafkaConfig
> 
> 
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec 
>   core/src/main/scala/kafka/Kafka.scala 77a49e12af6f869e63230162e9f87a7b0b12b610 
>   core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf 
>   core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 6d74983472249eac808d361344c58cc2858ec971 
>   core/src/main/scala/kafka/server/KafkaServer.scala 89200da30a04943f0b9befe84ab17e62b747c8c4 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 6879e730282185bda3d6bc3659cb15af0672cecf 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala e63558889272bc76551accdfd554bdafde2e0dd6 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala b15237b76def3b234924280fa3fdb25dbb0cc0dc 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 1bf2667f47853585bc33ffb3e28256ec5f24ae84 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86 
>   core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala a17e8532c44aadf84b8da3a57bcc797a848b5020 
>   core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala 95303e098d40cd790fb370e9b5a47d20860a6da3 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 25845abbcad2e79f56f729e59239b738d3ddbc9d 
>   core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala a5386a03b62956bc440b40783247c8cdf7432315 
>   core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala eab4b5f619015af42e4554660eafb5208e72ea33 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 35dc071b1056e775326981573c9618d8046e601d 
>   core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5 
>   core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala d6248b09bb0f86ee7d3bd0ebce5b99135491453b 
>   core/src/test/scala/unit/kafka/log/LogTest.scala c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 4ea0489c9fd36983fe190491a086b39413f3a9cd 
>   core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 3cf23b3d6d4460535b90cfb36281714788fc681c 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 1db6ac329f7b54e600802c8a623f80d159d4e69b 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala ce65dab4910d9182e6774f6ef1a7f45561ec0c23 
>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb 
>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala f0c4a56b61b4f081cf4bee799c6e9c523ff45e19 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala ad121169a5e80ebe1d311b95b219841ed69388e2 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 8913fc1d59f717c6b3ed12c8362080fb5698986b 
>   core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala a703d2715048c5602635127451593903f8d20576 
>   core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 82dce80d553957d8b5776a9e140c346d4e07f766 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala c06ee756bf0fe07e5d3c92823a476c960b37afd6 
>   core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala d5d351c4f25933da0ba776a6a89a989f1ca6a902 
>   core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 
>   core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala da4bafc1e2a94a436efe395aab1888fc21e55748 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala faa907131ed0aa94a7eacb78c1ffb576062be87a 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala cf2dd9455a9198b7215468df9f29568e91b31850 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala ba1e48e4300c9fb32e36e7266cb05294f2a481e5 
>   core/src/test/scala/unit/kafka/server/ServerStartupTest.scala 8fe7cd496f74ae1031019c2ca9ca013302294d9b 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala ccf5e2e36260b2484181b81d1b06e81de972674b 
>   core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala 84e08557de5acdcf0a98b192feac72836ea359b8 
> 
> Diff: https://reviews.apache.org/r/30126/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrii Biletskyi
> 
>