Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review92917 --- Ship it! Thanks for the patch. Looks good. +1 after addressing a few minor comments below. Also, could you rebase? core/src/main/scala/kafka/admin/ConfigCommand.scala (lines 134 - 135) https://reviews.apache.org/r/34554/#comment147186 Topics and Clients should be lower case. Better to reference the constants. core/src/main/scala/kafka/server/TopicConfigManager.scala (lines 35 - 36) https://reviews.apache.org/r/34554/#comment147183 I think it's better to have the type be topic and client, without s. core/src/main/scala/kafka/utils/ZkUtils.scala (line 769) https://reviews.apache.org/r/34554/#comment147185 Perhaps it's better to name this getAllEntitiesWithConfig()? - Jun Rao On July 17, 2015, 6:20 p.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 17, 2015, 6:20 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala 2b4e028f8a60fcae40c42cfabcc357e70e7ff9a6 core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 25, 2015, 1:12 a.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala 4cc237696c0630a67738e76cdbefb449975960c0 core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 4e28bf1c08414e8e96e6ca639b927d51bfeb4616 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 93f200e3e6de7dbb25c538a81137378cee84b94d core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 25, 2015, 1:11 a.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Minor changes Minor comments Diffs (updated) - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala 4cc237696c0630a67738e76cdbefb449975960c0 core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 4e28bf1c08414e8e96e6ca639b927d51bfeb4616 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 93f200e3e6de7dbb25c538a81137378cee84b94d core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
On July 17, 2015, 4:43 a.m., Jun Rao wrote: core/src/main/scala/kafka/server/ConfigHandler.scala, lines 64-65 https://reviews.apache.org/r/34554/diff/5/?file=1011630#file1011630line64 Could we just use Pool? Nice.. didn't know about that util. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review92008 --- On July 14, 2015, 5:37 p.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:37 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 17, 2015, 6:14 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 17, 2015, 6:14 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Diffs (updated) - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 17, 2015, 6:18 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Minor changes Diffs (updated) - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala 2b4e028f8a60fcae40c42cfabcc357e70e7ff9a6 core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 17, 2015, 6:20 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - bin/kafka-configs.sh PRE-CREATION core/src/main/scala/kafka/admin/AdminUtils.scala 2b4e028f8a60fcae40c42cfabcc357e70e7ff9a6 core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review92008 --- Thanks for the latest patch. Have a few more comments below. core/src/main/scala/kafka/admin/ConfigCommand.scala (line 34) https://reviews.apache.org/r/34554/#comment145852 Could we add a ConfigCommand shell script in bin/? core/src/main/scala/kafka/admin/ConfigCommand.scala (line 36) https://reviews.apache.org/r/34554/#comment145861 In topic command, if we don't provide the topic name, it will describe all topics. Could we add the same capability here? If entity name is not provided, we will just list all entities whose config has been overwritten. core/src/main/scala/kafka/admin/ConfigCommand.scala (lines 126 - 128) https://reviews.apache.org/r/34554/#comment145851 Currently, the output looks like the following. Could we put cleanup.policy on the next line? bin/kafka-run-class.sh kafka.admin.ConfigCommand Add/Remove entity (topic/client) configs Option Description -- --- --added-config Key Value pairs configs to add 'k1=v1, k2=v2'. The following is a list of valid configurations: For entity_type 'Topics': cleanup.policy compression.type delete.retention.ms file.delete.delay.ms flush.messages core/src/main/scala/kafka/admin/ConfigCommand.scala (line 159) https://reviews.apache.org/r/34554/#comment145859 entityType - entity_type core/src/main/scala/kafka/server/ConfigHandler.scala (lines 64 - 65) https://reviews.apache.org/r/34554/#comment145853 Could we just use Pool? core/src/main/scala/kafka/server/TopicConfigManager.scala (lines 35 - 36) https://reviews.apache.org/r/34554/#comment145860 The command line description says the entity types are topic/client, without s. We can reference the constants in the description as well. - Jun Rao On July 14, 2015, 5:37 p.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:37 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION
Re: Review Request 34554: Patch for KAFKA-2205
On July 9, 2015, 2:28 a.m., Jun Rao wrote: core/src/main/scala/kafka/server/ConfigHandler.scala, line 27 https://reviews.apache.org/r/34554/diff/3/?file=1001960#file1001960line27 Do we need JavaConversions? If this is needed, it would be better to import it in the context where the conversion is actually needed. Done. I'm using a ConcurrentMap to store the configs.. so I used the ConcurrentHashMap from java whic requires Java conversions - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review91038 --- On July 8, 2015, 2:14 a.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 8, 2015, 2:14 a.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala 20f1499046c768adbcd2bf8ad5969589c8641f34 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:36 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:37 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:34 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2186; Follow-up to KAFKA-1650 - add selective offset commit to consumer connector API; reviewed by Joel Koshy Revert KAFKA-2186; Follow-up to KAFKA-1650 - add selective offset commit to This reverts commit 23ff851f30bb55e794aefd2fae5367845d9230ee. KAFKA-1737; Enforce ZKSerializer while creating ZkClient; reviewed by Guozhang Wang kafka-2189; Snappy compression of message batches less efficient in 0.8.2.1; patched by Ismael Juma; reviewed by Jun Rao KAFKA-2186; Follow-up to KAFKA-1650 - add selective offset commit to consumer connector API; reviewed by Joel Koshy KAFKA-2091; Expose a partitioner interface in the new producer (https://cwiki.apache.org/confluence/display/KAFKA/KIP-+22+-+Expose+a+Partitioner+interface+in+the+new+producer); reviewed by Joel Koshy and Jay Kreps kafka-2185; Update to Gradle 2.4; patched by Ismael Juma; reviewed by Jun Rao KAFKA-2199 Make signing artifacts optional and disabled by default for SNAPSHOTs and allow remote Maven repository configuration from the command line. kafka-2226; NullPointerException in TestPurgatoryPerformance; patched by Yasuhiro Matsuda; reviewed by Onur Karaman, Guozhang Wang and Jun Rao KAFKA-2161; Fix a few copyrights KAFKA-2208; add consumer side error handling upon coordinator failure; reviewed by Onur Karaman kafka-1928; Move kafka.network over to using the network classes in org.apache.kafka.common.network; patched by Gwen Shapira; reviewed by Joel Koshy, Jay Kreps, Jiangjie Qin, Guozhang Wang and Jun Rao KAFKA-2246; UnknownTopicOrPartitionException should be an instance of InvalidMetadataException; reviewed by Ewen Cheslack-Postava and Joel Koshy KAFKA-2253; fix deadlock between removeWatchersLock and watcher operations list lock; reviewed by Onur Karaman and Jiangjie Qin kafka-2005; Generate html report for system tests; patched by Ashish Singh; reviewed by Jun Rao kafka-2266; Client Selector can drop idle connections without notifying NetworkClient; patched by Jason Gustafson; reviewed by Jun Rao kafka-2232; make MockProducer generic; patched by Alexander Pakulov; reviewed by Jun Rao kafka-2164; ReplicaFetcherThread: suspicious log message on reset offset; patched by Alexey Ozeritski; reviewed by Jun Rao kafka-2101; Metric metadata-age is reset on a failed update; patched by Tim Brooks; reviewed by Jun Rao kafka-2195; Add versionId to AbstractRequest.getErrorResponse and AbstractRequest.getRequest; patched by Andrii Biletskyi; reviewed by Jun Rao kafka-2270; incorrect package name in unit tests; patched by Proneet Verma; reviewed by Jun Rao kafka-2272; listeners endpoint parsing fails if the hostname has capital letter; patched by Sriharsha Chintalapani; reviewed by Jun Rao kafka-2264; SESSION_TIMEOUT_MS_CONFIG in ConsumerConfig should be int; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2252; Socket connection closing is logged, but not corresponding opening of socket; patched by Gwen Shapira; reviewed by Jun Rao kafka-2262; LogSegmentSize validation should be consistent; patched by Manikumar Reddy; reviewed by Jun Rao trivial fix for stylecheck error on Jenkins kafka-2249; KafkaConfig does not preserve original Properties; patched by Gwen Shapira; reviewed by Jun Rao kafka-2265; creating a topic with large number of partitions takes a long time; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2234; Partition reassignment of a nonexistent topic prevents future reassignments; patched by Manikumar Reddy; reviewed by Jun Rao trivial change to fix unit test failure introduced in kafka-2234 kafka-1758; corrupt recovery file prevents startup; patched by Manikumar Reddy; reviewed by Neha Narkhede and Jun Rao kafka-1646; Improve consumer read performance for Windows; patched by Honghai Chen; reviewed by Jay Kreps and Jun Rao kafka-2012; Broker should automatically handle corrupt index files; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2290; OffsetIndex should open RandomAccessFile consistently; patched by Chris Black; reviewed by Jun Rao kafka-2235; LogCleaner offset map overflow; patched by Ivan Simoneko; reviewed by Jun Rao KAFKA-2245; Add response tests for consumer coordinator; reviewed by Joel Koshy KAFKA-2293; Fix incorrect format specification in Partition.scala; reviewed by Joel Koshy kafka-2168; New consumer poll() can block other calls like position(), commit(), and close() indefinitely; patched by Jason Gustafson; reviewed by Jay Kreps, Ewen Cheslack-Postava, Guozhang Wang and
Re: Review Request 34554: Patch for KAFKA-2205
On July 9, 2015, 2:28 a.m., Jun Rao wrote: core/src/main/scala/kafka/admin/ConfigCommand.scala, line 123 https://reviews.apache.org/r/34554/diff/3/?file=1001954#file1001954line123 Could we list the valid configs name for each entity-type as we did in TopicCommand? I've listed the valid configs for entity_type: Topics. 'Clients' is currently empty.. I'll populate it once I publish the patch for changing quota metrics dynamically. That requires KAFKA-2084 and 2136 to be committed first. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review91038 --- On July 14, 2015, 5:37 p.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 14, 2015, 5:37 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a90aa8787ff21b963765a547980154363c1c93c6 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review91038 --- Thanks for the new patch. Just a couple of minor comments below. core/src/main/scala/kafka/admin/ConfigCommand.scala (line 123) https://reviews.apache.org/r/34554/#comment144291 Could we list the valid configs name for each entity-type as we did in TopicCommand? core/src/main/scala/kafka/server/ConfigHandler.scala (line 27) https://reviews.apache.org/r/34554/#comment144282 Do we need JavaConversions? If this is needed, it would be better to import it in the context where the conversion is actually needed. - Jun Rao On July 8, 2015, 2:14 a.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 8, 2015, 2:14 a.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala 20f1499046c768adbcd2bf8ad5969589c8641f34 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
On July 7, 2015, 2:18 a.m., Jun Rao wrote: Thanks for the patch. A few more comments below. 1. The patch doesn't apply. Could you rebase? 2. Also, we need the logic to read all existing client configs. Is that in a separate jira? Aditya Auradkar wrote: 1. Will do. 2. Hey Jun - I didn't understand what you meant by read all existing client configs. Can you elaborate? In general, I'm submitting all followup client config changes in subsequent patches to avoid making the patches too large. This patch will basically refactor the code and make it possible to receive client change notifications. 2. When starting up a broker, we need to read all existing client configs into ClientIdConfigHandler, right? - Jun --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review90622 --- On July 8, 2015, 2:14 a.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 8, 2015, 2:14 a.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala 20f1499046c768adbcd2bf8ad5969589c8641f34 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 8, 2015, 2:12 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Addressing Jun's comments Addressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala 20f1499046c768adbcd2bf8ad5969589c8641f34 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
On July 7, 2015, 2:18 a.m., Jun Rao wrote: Thanks for the patch. A few more comments below. 1. The patch doesn't apply. Could you rebase? 2. Also, we need the logic to read all existing client configs. Is that in a separate jira? 1. Will do. 2. Hey Jun - I didn't understand what you meant by read all existing client configs. Can you elaborate? In general, I'm submitting all followup client config changes in subsequent patches to avoid making the patches too large. This patch will basically refactor the code and make it possible to receive client change notifications. On July 7, 2015, 2:18 a.m., Jun Rao wrote: core/src/main/scala/kafka/admin/ConfigCommand.scala, line 49 https://reviews.apache.org/r/34554/diff/2/?file=997805#file997805line49 What is the 1 at the end? typo. On July 7, 2015, 2:18 a.m., Jun Rao wrote: core/src/main/scala/kafka/server/TopicConfigManager.scala, line 157 https://reviews.apache.org/r/34554/diff/2/?file=997814#file997814line157 It's probably useful to include the orginal json string. Also, could we make the message string in all IllegalArgumentException consistent? For example, they should all reference config change notification. Improved the error messages a bit On July 7, 2015, 2:18 a.m., Jun Rao wrote: core/src/main/scala/kafka/admin/ConfigCommand.scala, lines 60-106 https://reviews.apache.org/r/34554/diff/2/?file=997805#file997805line60 It seems that all those methods can be private. parseConfigsToBeAdded and parseConfigsToBeDeleted are used in ConfigCommandTest. I've made them private to the admin package. On July 7, 2015, 2:18 a.m., Jun Rao wrote: core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala, lines 88-96 https://reviews.apache.org/r/34554/diff/2/?file=997819#file997819line88 Are we really mocking zkclient calls here? Nope.. no zkclient calls being mocked here. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review90622 --- On July 2, 2015, 1:39 a.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 2, 2015, 1:39 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205. Summary of changes: 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b core/src/main/scala/kafka/cluster/Partition.scala 730a232482fdf77be5704cdf5941cfab3828db88 core/src/main/scala/kafka/controller/KafkaController.scala 69bba243a9a511cc5292b43da0cc48e421a428b0 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/TopicConfigManager.scala b675a7e45ea4f4179f8b15fe221fd988aff13aa0 core/src/main/scala/kafka/utils/ZkUtils.scala 2618dd39b925b979ad6e4c0abd5c6eaafb3db5d5 core/src/test/scala/unit/kafka/admin/AdminTest.scala efb2f8e79b3faef78722774b951fea828cd50374 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala c7136f20972614ac47aa57ab13e3c94ef775a4b7 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 7877f6ca1845c2edbf96d4a9783a07a552db8f07 Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 8, 2015, 2:13 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205: Summary of changes 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly 6. Addressed all of Jun's comments. Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 core/src/main/scala/kafka/cluster/Partition.scala 2649090b6cbf8d442649f19fd7113a30d62bca91 core/src/main/scala/kafka/controller/KafkaController.scala 20f1499046c768adbcd2bf8ad5969589c8641f34 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8764522e7947bb08998256ce1deb717c84 core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b5976ffa34cafdae261239934b1b917bfe core/src/main/scala/kafka/server/TopicConfigManager.scala 01b1b0a8efe6ab3ddc7bf9f1f535b01be4e2e6be core/src/main/scala/kafka/utils/ZkUtils.scala 166814c2959a429e20f400d1c0e523090ce37d91 core/src/test/scala/unit/kafka/admin/AdminTest.scala 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala dcd69881445c29765f66a7d21d2d18437f4df428 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 8a871cfaf6a534acd1def06a5ac95b5c985b024c Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/#review90622 --- Thanks for the patch. A few more comments below. 1. The patch doesn't apply. Could you rebase? 2. Also, we need the logic to read all existing client configs. Is that in a separate jira? core/src/main/scala/kafka/admin/ConfigCommand.scala (line 49) https://reviews.apache.org/r/34554/#comment143735 What is the 1 at the end? core/src/main/scala/kafka/admin/ConfigCommand.scala (lines 60 - 106) https://reviews.apache.org/r/34554/#comment143736 It seems that all those methods can be private. core/src/main/scala/kafka/admin/TopicCommand.scala (line 243) https://reviews.apache.org/r/34554/#comment143740 The description needs to be changed to reflect what can now be altered. core/src/main/scala/kafka/admin/TopicCommand.scala (line 252) https://reviews.apache.org/r/34554/#comment143739 Since topic config can't be altered here, we need to change the description accordingly. core/src/main/scala/kafka/admin/TopicCommand.scala (line 258) https://reviews.apache.org/r/34554/#comment143741 Since we can't alter the config, it seems that we don't need this option at all. core/src/main/scala/kafka/server/ConfigHandler.scala (line 28) https://reviews.apache.org/r/34554/#comment143742 case k: (TopicAndPartition, Log) = k._1.topic can just be case (topicAndPartition, log) = topicAndPartition.topic core/src/main/scala/kafka/server/ConfigHandler.scala (line 31) https://reviews.apache.org/r/34554/#comment143743 Not needed? core/src/main/scala/kafka/server/KafkaServer.scala (line 31) https://reviews.apache.org/r/34554/#comment143750 Do we need JavaConversions? If this is needed, it would be better to import it in the context where the conversion is actually needed. core/src/main/scala/kafka/server/TopicConfigManager.scala (line 133) https://reviews.apache.org/r/34554/#comment143745 entityType = entity_type. Also, it would be useful to include the original json. core/src/main/scala/kafka/server/TopicConfigManager.scala (line 138) https://reviews.apache.org/r/34554/#comment143746 Value = entity_name core/src/main/scala/kafka/server/TopicConfigManager.scala (lines 140 - 143) https://reviews.apache.org/r/34554/#comment143747 Since we have verified the entity type before, we don't need to check the handler exists or not here. core/src/main/scala/kafka/server/TopicConfigManager.scala (line 145) https://reviews.apache.org/r/34554/#comment143748 It's probably useful to include the orginal json string. Also, could we make the message string in all IllegalArgumentException consistent? For example, they should all reference config change notification. core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala (lines 88 - 96) https://reviews.apache.org/r/34554/#comment143751 Are we really mocking zkclient calls here? - Jun Rao On July 2, 2015, 1:39 a.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 2, 2015, 1:39 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description --- KAFKA-2205. Summary of changes: 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b core/src/main/scala/kafka/cluster/Partition.scala 730a232482fdf77be5704cdf5941cfab3828db88 core/src/main/scala/kafka/controller/KafkaController.scala 69bba243a9a511cc5292b43da0cc48e421a428b0 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 2, 2015, 1:38 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- Some fixes KAFKA-2205 KAFKA-2205 Addressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b core/src/main/scala/kafka/cluster/Partition.scala 730a232482fdf77be5704cdf5941cfab3828db88 core/src/main/scala/kafka/controller/KafkaController.scala 69bba243a9a511cc5292b43da0cc48e421a428b0 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/TopicConfigManager.scala b675a7e45ea4f4179f8b15fe221fd988aff13aa0 core/src/main/scala/kafka/utils/ZkUtils.scala 2618dd39b925b979ad6e4c0abd5c6eaafb3db5d5 core/src/test/scala/unit/kafka/admin/AdminTest.scala efb2f8e79b3faef78722774b951fea828cd50374 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala c7136f20972614ac47aa57ab13e3c94ef775a4b7 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 7877f6ca1845c2edbf96d4a9783a07a552db8f07 Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated July 2, 2015, 1:39 a.m.) Review request for kafka and Joel Koshy. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205. Summary of changes: 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b core/src/main/scala/kafka/cluster/Partition.scala 730a232482fdf77be5704cdf5941cfab3828db88 core/src/main/scala/kafka/controller/KafkaController.scala 69bba243a9a511cc5292b43da0cc48e421a428b0 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/TopicConfigManager.scala b675a7e45ea4f4179f8b15fe221fd988aff13aa0 core/src/main/scala/kafka/utils/ZkUtils.scala 2618dd39b925b979ad6e4c0abd5c6eaafb3db5d5 core/src/test/scala/unit/kafka/admin/AdminTest.scala efb2f8e79b3faef78722774b951fea828cd50374 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala c7136f20972614ac47aa57ab13e3c94ef775a4b7 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 7877f6ca1845c2edbf96d4a9783a07a552db8f07 Diff: https://reviews.apache.org/r/34554/diff/ Testing --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
Re: Review Request 34554: Patch for KAFKA-2205
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34554/ --- (Updated May 21, 2015, 5:55 p.m.) Review request for kafka. Bugs: KAFKA-2205 https://issues.apache.org/jira/browse/KAFKA-2205 Repository: kafka Description (updated) --- KAFKA-2205. Summary of changes: 1. Generalized TopicConfigManager to DynamicConfigManager. It is now able to handle multiple types of entities. 2. Changed format of the notification znode as described in KIP-21 3. Replaced TopicConfigManager with DynamicConfigManager. 4. Added new testcases. Existing testcases all pass 5. Added ConfigCommand to handle all config changes. Eventually this will make calls to the broker once the new API's are built for now it speaks to ZK directly Diffs - core/src/main/scala/kafka/admin/AdminUtils.scala f06edf41c732a7b794e496d0048b0ce6f897e72b core/src/main/scala/kafka/admin/ConfigCommand.scala PRE-CREATION core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b core/src/main/scala/kafka/cluster/Partition.scala 730a232482fdf77be5704cdf5941cfab3828db88 core/src/main/scala/kafka/controller/KafkaController.scala 69bba243a9a511cc5292b43da0cc48e421a428b0 core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f core/src/main/scala/kafka/controller/TopicDeletionManager.scala 64ecb499f24bc801d48f86e1612d927cc08e006d core/src/main/scala/kafka/server/ConfigHandler.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaServer.scala ea6d165d8e5c3146d2c65e8ad1a513308334bf6f core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/TopicConfigManager.scala b675a7e45ea4f4179f8b15fe221fd988aff13aa0 core/src/main/scala/kafka/utils/ZkUtils.scala 2618dd39b925b979ad6e4c0abd5c6eaafb3db5d5 core/src/test/scala/unit/kafka/admin/AdminTest.scala efb2f8e79b3faef78722774b951fea828cd50374 core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala PRE-CREATION core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala c7136f20972614ac47aa57ab13e3c94ef775a4b7 core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 7877f6ca1845c2edbf96d4a9783a07a552db8f07 Diff: https://reviews.apache.org/r/34554/diff/ Testing (updated) --- 1. Added new testcases for new code. 2. Verified that both topic and client configs can be changed dynamically by starting a local cluster Thanks, Aditya Auradkar
