Kafka topology monitoring

[timothy]

Good Afternoon and hope you all are well. 

Im trying to monitor my topologies to ensure That I can look at the
following metrics 

Thread metrics 

- Average time for commits, poll, process operations 

- Tasks created per second, tasked closed per second 

I have seen metrics in micrometer, but they don't seem to extrapolate
the above messages, is there a tool that I can use to get this level of
detailed monitoring? 

best regards 

Timothy

Re: Fault Injection

[Timothy Chen]

Hi Colin,

The Kibosh code is just a README for now, is it going to be published soon?

Tim

On Tue, Aug 22, 2017 at 11:44 AM, Colin McCabe  wrote:
> Hi all,
>
> I've been working on a fault injector for Apache Kafka.  The general
> idea is to create faults such as network partitions or disk failures,
> and see what happens in the cluster.  The fault injector can run as part
> of a ducktape system test, or standalone.
>
> The fault injector has two processes: a coordinator, and an agent.  The
> agent process is responsible for actually implementing the faults.  For
> example, it might run iptables, send signals to processes, generate a
> lot of load, or do something else to disrupt the computer it is running
> on.  We run an agent process on each node where we would like to
> potentially inject faults.  So it will run alongside the brokers,
> zookeeper nodes, etc.
>
> The coordinator process is responsible for communicating with the agent
> processes and for scheduling faults.  For example, the coordinator can
> be instructed to create a fault immediately on several nodes.  Or it can
> be instructed to create faults over time, based on a pseudorandom seed.
> Both the coordinator and the agent expose a REST interface that accepts
> objects serialized via JSON.
>
> I think two kinds of faults will be especially interesting: network
> faults, and disk errors.  Simulating network faults in a Linux
> environment is relatively straightforward using iptables.  Disk errors
> are tougher to simulate, but I have written a FUSE filesystem to do
> this.  The  filesystem essentially simulates a bind mount in most cases,
> but it can take a JSON specification telling it to inject certain
> faults.  (Disk errors seem especially relevant to the ongoing work on
> JBOD.)
>
> Although it's not a user-visible component, I think having a fault
> injector will be really great for Kafka users.  It will really help us
> stress test Kafka in more situations.  I'm going to post some patches in
> a day or two-- it would be great to get some feedback.  Check out
> https://cwiki.apache.org/confluence/display/KAFKA/Fault+Injection
>
> best,
> Colin

Re: Guozhang Wang elected to Kafka PMC

[Timothy Chen]

Congrats Guozhang!!

Tim

On Tue, Jun 16, 2015 at 8:19 AM, Aditya Auradkar
 wrote:
> Congrats Guozhang!
>
> 
> From: Ashish Singh [asi...@cloudera.com]
> Sent: Monday, June 15, 2015 10:53 PM
> To: dev@kafka.apache.org
> Cc: Jun Rao
> Subject: Re: Guozhang Wang elected to Kafka PMC
>
> Congrats Guozhang!
>
> On Mon, Jun 15, 2015 at 10:20 PM, Sriharsha Chintalapani 
> wrote:
>
>> Congrats Guozhang.
>>
>> --
>> Harsha
>>
>>
>> On June 15, 2015 at 9:59:37 PM, Jun Rao (j...@confluent.io) wrote:
>>
>> Hi, Everyone,
>>
>> Guozhang Wang has been active in the Kafka community since he became a
>> Kafka committer about 7 months ago. I am glad to announce that Guozhang is
>> now a member of Kafka PMC.
>>
>> Congratulations, Guozhang!
>>
>> Jun
>>
>
>
>
> --
>
> Regards,
> Ashish

Review Request 36474: Patch for KAFKA-2188

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-2188 - JBOD Support


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
2649090b6cbf8d442649f19fd7113a30d62bca91 
  core/src/main/scala/kafka/common/GenericKafkaStorageException.scala 
PRE-CREATION 
  core/src/main/scala/kafka/controller/KafkaController.scala 
b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 
bb6b5c8764522e7947bb08998256ce1deb717c84 
  core/src/main/scala/kafka/log/Log.scala 
e5e80079645ce6e6fe7bb1c2696d3dd21a07761b 
  core/src/main/scala/kafka/log/LogManager.scala 
69386c17153e5ef08a24d4f14b915e4316b121d8 
  core/src/main/scala/kafka/log/LogSegment.scala 
1377e8f322a3fedc683d93feaf27c955de528a4b 
  core/src/main/scala/kafka/server/KafkaApis.scala 
18f5b5b895af1469876b2223841fd90a2dd255e0 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
dbe170f87331f43e2dc30165080d2cb7dfe5fdbf 
  core/src/main/scala/kafka/server/OffsetCheckpoint.scala 
8c5b0546908d3b3affb9f48e2ece9ed252518783 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
c89d00b5976ffa34cafdae261239934b1b917bfe 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
795220e7f63d163be90738b4c1a39687b44c1395 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
166814c2959a429e20f400d1c0e523090ce37d91 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
a13f2bef8ee8c3d42192c9a60df092023e4d2ff9 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
60cd8249e6ec03349e20bb0a7226ea9cd66e6b17 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
17e9fe4c159a29033fe9a287db6ced2fdc3fa9d1 

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


Testing
---


Thanks,

Timothy Chen

Review Request 36503: Patch for KAFKA-2188

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-2188 - JBOD Support


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
2649090b6cbf8d442649f19fd7113a30d62bca91 
  core/src/main/scala/kafka/common/GenericKafkaStorageException.scala 
PRE-CREATION 
  core/src/main/scala/kafka/controller/KafkaController.scala 
b4fc755641b9bbe8a6bf9c221a9ffaec0b94d6e8 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 
bb6b5c8764522e7947bb08998256ce1deb717c84 
  core/src/main/scala/kafka/log/Log.scala 
e5e80079645ce6e6fe7bb1c2696d3dd21a07761b 
  core/src/main/scala/kafka/log/LogManager.scala 
69386c17153e5ef08a24d4f14b915e4316b121d8 
  core/src/main/scala/kafka/log/LogSegment.scala 
1377e8f322a3fedc683d93feaf27c955de528a4b 
  core/src/main/scala/kafka/server/KafkaApis.scala 
18f5b5b895af1469876b2223841fd90a2dd255e0 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
dbe170f87331f43e2dc30165080d2cb7dfe5fdbf 
  core/src/main/scala/kafka/server/OffsetCheckpoint.scala 
8c5b0546908d3b3affb9f48e2ece9ed252518783 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
c89d00b5976ffa34cafdae261239934b1b917bfe 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
795220e7f63d163be90738b4c1a39687b44c1395 
  core/src/main/scala/kafka/utils/CoreUtils.scala 
168a18d380c200ee566eccb6988dd1ae85ed5b09 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
166814c2959a429e20f400d1c0e523090ce37d91 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
a13f2bef8ee8c3d42192c9a60df092023e4d2ff9 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
60cd8249e6ec03349e20bb0a7226ea9cd66e6b17 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
00d59337a99ac135e8689bd1ecd928f7b1423d79 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
17e9fe4c159a29033fe9a287db6ced2fdc3fa9d1 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 36474: Patch for KAFKA-2188

[Timothy Chen]

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

(Updated July 15, 2015, 8:19 a.m.)


Review request for kafka and Joe Stein.


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


Repository: kafka


Description
---

KAFKA-2188 - JBOD Support


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 2649090 
  core/src/main/scala/kafka/common/GenericKafkaStorageException.scala 
PRE-CREATION 
  core/src/main/scala/kafka/controller/KafkaController.scala b4fc755 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala bb6b5c8 
  core/src/main/scala/kafka/log/Log.scala e5e8007 
  core/src/main/scala/kafka/log/LogManager.scala 69386c1 
  core/src/main/scala/kafka/log/LogSegment.scala 1377e8f 
  core/src/main/scala/kafka/server/KafkaApis.scala 18f5b5b 
  core/src/main/scala/kafka/server/KafkaConfig.scala dbe170f 
  core/src/main/scala/kafka/server/OffsetCheckpoint.scala 8c5b054 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala c89d00b 
  core/src/main/scala/kafka/server/ReplicaManager.scala 795220e 
  core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d 
  core/src/main/scala/kafka/utils/ZkUtils.scala 166814c 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala a13f2be 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
60cd824 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 00d5933 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 17e9fe4 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19379: Patch for KAFKA-1311

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On March 19, 2014, 12:14 a.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19379/
> ---
> 
> (Updated March 19, 2014, 12:14 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1311
> https://issues.apache.org/jira/browse/KAFKA-1311
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Joel's review comments
> 
> 
> 1. Removed --delete from topic command 2. Added config to turn off delete 
> topic (delete.topic.enable)
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 
> 6fef9df19fbc7caf90d0409bffd36fea1f4c4da5 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala 
> c69077e6f4dc04232f63d748aa4e49d33d0cebc4 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> 58f1c4274e9f8ec3f4711974eae0588020c5c169 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> 08de0eff67311e25d56d4133882faea6758c977c 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 6db76a53173aab06b84203712c98313ce1e8b12e 
> 
> Diff: https://reviews.apache.org/r/19379/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19388: KAFKA-1251: Add metrics to the producer.

[Timothy Chen]

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



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/19388/#comment69332>

Not sure if this is intentional.



clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java
<https://reviews.apache.org/r/19388/#comment69333>

Should all the callers also check null now?


- Timothy Chen


On March 19, 2014, 4:07 a.m., Jay Kreps wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19388/
> ---
> 
> (Updated March 19, 2014, 4:07 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1251
> https://issues.apache.org/jira/browse/KAFKA-1251
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> This is a draft patch so we can discuss what to instrument.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 1ac69436f117800815b8d50f042e9e2a29364b43 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java
>  33d62a4b83fbab5b22b91b22f6b744af1c98d262 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  673b2962771c28ceb3c7a6c0fd6f69521bd7ed16 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  038a05a94b795ec0a95b2d40a89222394b5a74c4 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 565331dfb9cd1d65be37ed97830aa42e44d2e127 
>   
> clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 
> 3ebbb804242be6a001b3bae6524afccc85a87602 
>   clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 
> 7e4849b7a148009c8a878349d7f0239108ccad8c 
>   clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 
> 3b0454f26490d1f4a2a80efb00165fc72587fbf8 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 
> f8b413a8c273cdad56177fbc6971fece4feb86b3 
>   clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 
> 9305b61ddeaa2bb400cbbb6d3c99c8ecaade6b8f 
>   clients/src/main/java/org/apache/kafka/common/network/NetworkReceive.java 
> 51d4892dfc18580e5e213d386c5de387a47d3c6b 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 983963200ce81614577cd6182a5d2f10c22b95d4 
>   clients/src/test/java/org/apache/kafka/clients/producer/MetadataTest.java 
> 09a5355d25a3b94c8e23caa2adc77cb1c59368b9 
>   
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java
>  ed5690641a22fbe4bd91b0c6055d465944b08c06 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 
> 12c9500ce4387306ab5aa7a5781b4aca52b86604 
>   clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
> 90e2dcf5434db546387302fb0219edfdb363592e 
>   clients/src/test/java/org/apache/kafka/test/MetricsBench.java 
> 7239b4a56e93f019e66aa2cf2aa9b04c26908bfd 
> 
> Diff: https://reviews.apache.org/r/19388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>

Re: Review Request 19388: Patch for KAFKA-1251

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On March 19, 2014, 5:29 p.m., Jay Kreps wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19388/
> ---
> 
> (Updated March 19, 2014, 5:29 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1251
> https://issues.apache.org/jira/browse/KAFKA-1251
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Reviewable patch.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 1ac69436f117800815b8d50f042e9e2a29364b43 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java
>  33d62a4b83fbab5b22b91b22f6b744af1c98d262 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  673b2962771c28ceb3c7a6c0fd6f69521bd7ed16 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  038a05a94b795ec0a95b2d40a89222394b5a74c4 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 565331dfb9cd1d65be37ed97830aa42e44d2e127 
>   
> clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 
> 3ebbb804242be6a001b3bae6524afccc85a87602 
>   clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 
> 6db2dfbe94c940efa37463298f0b0b1893e646e1 
>   clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 
> 7e4849b7a148009c8a878349d7f0239108ccad8c 
>   clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 
> 3b0454f26490d1f4a2a80efb00165fc72587fbf8 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java 
> f8b413a8c273cdad56177fbc6971fece4feb86b3 
>   clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java 
> 9305b61ddeaa2bb400cbbb6d3c99c8ecaade6b8f 
>   clients/src/main/java/org/apache/kafka/common/network/NetworkReceive.java 
> 51d4892dfc18580e5e213d386c5de387a47d3c6b 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 983963200ce81614577cd6182a5d2f10c22b95d4 
>   clients/src/test/java/org/apache/kafka/clients/producer/MetadataTest.java 
> 09a5355d25a3b94c8e23caa2adc77cb1c59368b9 
>   
> clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java
>  ed5690641a22fbe4bd91b0c6055d465944b08c06 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java 
> 12c9500ce4387306ab5aa7a5781b4aca52b86604 
>   clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
> 90e2dcf5434db546387302fb0219edfdb363592e 
>   clients/src/test/java/org/apache/kafka/test/MetricsBench.java 
> 7239b4a56e93f019e66aa2cf2aa9b04c26908bfd 
> 
> Diff: https://reviews.apache.org/r/19388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>

Review Request 19421: Patch for KAFKA-1312

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1312 Add IDE generated files into git ignore


Diffs
-

  .gitignore 553a077d031a37d78f4921d9e7ab39132d979276 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 18299: Fix KAFKA-1253

[Timothy Chen]

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



clients/src/main/java/org/apache/kafka/common/record/ByteBufferOutputStream.java
<https://reviews.apache.org/r/18299/#comment69589>

Perhaps only provide a getter to avoid the stream state getting modified.


- Timothy Chen


On March 19, 2014, 11:32 p.m., Guozhang Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18299/
> ---
> 
> (Updated March 19, 2014, 11:32 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1253
> https://issues.apache.org/jira/browse/KAFKA-1253
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Incorporated Jay's comments
> 
> In-place compression with
> 
> 1) Dynamic reallocation in the underlying byte buffer
> 2) Written bytes estimate to reduce reallocation probabilities
> 3) Deallocation in buffer pool following the original capacity
> 
> 
> Diffs
> -
> 
>   build.gradle 84fa0d6b5f7405af755c5d7ff7bdd7592bb8668f 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 1ac69436f117800815b8d50f042e9e2a29364b43 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> 32e12ad149f6d70c96a498d0a390976f77bf9e2a 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java
>  b69866a9fb9a8b4e1e78d304a20eda3cbf178c6f 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
>  673b2962771c28ceb3c7a6c0fd6f69521bd7ed16 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java
>  038a05a94b795ec0a95b2d40a89222394b5a74c4 
>   
> clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java 
> 3ebbb804242be6a001b3bae6524afccc85a87602 
>   
> clients/src/main/java/org/apache/kafka/common/record/ByteBufferInputStream.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/record/ByteBufferOutputStream.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/record/CompressionType.java 
> 906da02d02c03aadd8ab73ed2fc9a1898acb8d72 
>   clients/src/main/java/org/apache/kafka/common/record/Compressor.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 
> 9d8935fa3beeb2a78b109a41ed76fd4374239560 
>   clients/src/main/java/org/apache/kafka/common/record/Record.java 
> f1dc9778502cbdfe982254fb6e25947842622239 
>   clients/src/main/java/org/apache/kafka/common/utils/Crc32.java 
> 153c5a6d345293aa0ba2cf513373323a6e9f2467 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> 0c6b3656375721a718fb4de10118170aacce0ea9 
>   clients/src/test/java/org/apache/kafka/common/record/MemoryRecordsTest.java 
> b0745b528cef929c4273f7e2ac4de1476cfc25ad 
>   clients/src/test/java/org/apache/kafka/common/record/RecordTest.java 
> ae54d67da9907b0a043180c7395a1370b3d0528d 
>   clients/src/test/java/org/apache/kafka/common/utils/CrcTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/test/TestUtils.java 
> 36cfc0fda742eb501af2c2c0330e3f461cf1f40c 
>   core/src/main/scala/kafka/producer/ConsoleProducer.scala 
> dd39ff22c918fe5b05f04582b748e32349b2055f 
>   core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala 
> PRE-CREATION 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
> c002f5ea38ece66ad559fadb18ffaf40ac2026aa 
>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 
> 66ea76b9b6c0f8839f715c845fb9b9671b8f35c1 
>   perf/src/main/scala/kafka/perf/ProducerPerformance.scala 
> 3df0d130308a35fca96184adc212eea1488f 
> 
> Diff: https://reviews.apache.org/r/18299/diff/
> 
> 
> Testing
> ---
> 
> integration tests
> 
> unit tests
> 
> stress tests (1K message size, 1M messages in producer performance with ack = 
> 1, linger time = 0ms/500ms, random bit/all-ones messages)
> 
> snappy dynamic load test
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Review Request 19490: Patch for KAFKA-1315

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1315 Fix ReplicaManager directory matching in checkpointing


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
f16fbe672861cb3ea866f80a835e7e502aa9c6ec 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
02c188a412995f876dd6616d70027b84c07eb264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala PRE-CREATION 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19490: Patch for KAFKA-1315

[Timothy Chen]

> On March 20, 2014, 8:20 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/cluster/Partition.scala, line 93
> > <https://reviews.apache.org/r/19490/diff/1/?file=530482#file530482line93>
> >
> > This creates new File objects on every single getOrCreateReplica. 
> > During leader changes, that would create too many File objects all at once. 
> > If the purpose is to get a standardized File name, can we just do new 
> > File(dir).getName when we put the entry in highWatermarkCheckpoints?

Makes sense! Had the same thought briefly but didn't know the frequency of the 
call.


- Timothy


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


On March 20, 2014, 7:49 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19490/
> ---
> 
> (Updated March 20, 2014, 7:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1315
> https://issues.apache.org/jira/browse/KAFKA-1315
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1315 Fix ReplicaManager directory matching in checkpointing
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 0b88f14c4855b27242906cd45930bae501e26226 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> f16fbe672861cb3ea866f80a835e7e502aa9c6ec 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
> 02c188a412995f876dd6616d70027b84c07eb264 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19490/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19490: Patch for KAFKA-1315

[Timothy Chen]

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

(Updated March 20, 2014, 8:28 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1315 Fix ReplicaManager directory matching in checkpointing


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
f16fbe672861cb3ea866f80a835e7e502aa9c6ec 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
02c188a412995f876dd6616d70027b84c07eb264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala PRE-CREATION 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19490: Patch for KAFKA-1315

[Timothy Chen]

> On March 20, 2014, 8:41 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/cluster/Partition.scala, line 93
> > <https://reviews.apache.org/r/19490/diff/2/?file=530513#file530513line93>
> >
> > Don't you still need to change this place?

Don't need to change this as getAbsolutePath matches the getParent folder path 
string.


- Timothy


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


On March 20, 2014, 8:28 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19490/
> ---
> 
> (Updated March 20, 2014, 8:28 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1315
> https://issues.apache.org/jira/browse/KAFKA-1315
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1315 Fix ReplicaManager directory matching in checkpointing
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 0b88f14c4855b27242906cd45930bae501e26226 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> f16fbe672861cb3ea866f80a835e7e502aa9c6ec 
>   core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
> 02c188a412995f876dd6616d70027b84c07eb264 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19490/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19490: Patch for KAFKA-1315

[Timothy Chen]

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

(Updated March 20, 2014, 9:03 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1315 Fix ReplicaManager directory matching in checkpointing


Diffs (updated)
-

  core/src/main/scala/kafka/server/ReplicaManager.scala 
f16fbe672861cb3ea866f80a835e7e502aa9c6ec 
  core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala 
02c188a412995f876dd6616d70027b84c07eb264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala PRE-CREATION 

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


Testing
---


Thanks,

Timothy Chen

Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 24, 2014, 6:48 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

> On March 24, 2014, 4:20 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, line 74
> > <https://reviews.apache.org/r/19577/diff/2/?file=533764#file533764line74>
> >
> > This is probably trying to fix the problem reported in KAFKA-1307? If 
> > yes, then this change might not be enough to fix the issue. Since the 
> > await() and signal would now have to ensure that they lock on deleteLock 
> > instead of controllerLock.

This is not intended to fix KAFKA-1307. I actually got a deadlock when I just 
call deleteTopicThread.shutdown in controller shutdown, and took me sometime to 
finally figure out that I was holding the controllerLock while trying to 
shutdown the deleteTopicThread, which the thread is awaiting on the same lock's 
condition. I don't see why the delete Topic manager needs to hold on to the 
same lock, as the signaling and awaiting seems to be indepedendent of other 
operations (but I very well can be wrong).

Looking at KAFKA-1307 it seems that a patch is submitted for that already? Do 
we still see issues from that?


- Timothy


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


On March 24, 2014, 6:48 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19577/
> ---
> 
> (Updated March 24, 2014, 6:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1317
> https://issues.apache.org/jira/browse/KAFKA-1317
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1317 Fix Kafka shutdown with delete topic on
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> 8262e10ce4364f057e449172ace38320e44247db 
> 
> Diff: https://reviews.apache.org/r/19577/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 24, 2014, 6:06 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 

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


Testing
---


Thanks,

Timothy Chen

Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1323 Fix log directory to support relative directories


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
255be063ee247a849e527297c987da4625d749ca 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 25, 2014, 10:20 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

> On March 26, 2014, 12:55 a.m., Neha Narkhede wrote:
> > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala, line 66
> > <https://reviews.apache.org/r/19626/diff/1/?file=535572#file535572line66>
> >
> > This method can be private.
> > 
> > This method seems to indicate that it creates a mock log manager, but 
> > actually it creates a real LogManager object. I'm not sure I understand the 
> > motivation behind moving away from the mock object to an actual LogManager 
> > object?

Originally I just mocked LogManager with EasyMock, but encountered the problem 
where Partition.getOrCreateReplica is looking for properties in LogManager and 
seems like EasyMock only allows mocking methods not public properties.

It's not a full mock, but I'm using a mocked object (MockTime) so I named it 
mock log manager. Can definitely just rename the test method and make it 
private.


- Timothy


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


On March 25, 2014, 6:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19626/
> ---
> 
> (Updated March 25, 2014, 6:54 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1323
> https://issues.apache.org/jira/browse/KAFKA-1323
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1323 Fix log directory to support relative directories
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 0b88f14c4855b27242906cd45930bae501e26226 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 255be063ee247a849e527297c987da4625d749ca 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
> b5936d4101b513baa805ab26361fe965bdf980aa 
> 
> Diff: https://reviews.apache.org/r/19626/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

> On March 26, 2014, 12:55 a.m., Neha Narkhede wrote:
> > My guess is that we may have to review and make a similar change in the 
> > following places -
> > 1. LogManager.checkpointRecoveryPointOffsets()
> > 2. LogManager.nextLogDir()
> > 
> > The unit test in ReplicaManagerTest cover checkpointing, but we also need 
> > to add coverage for log recovery and log cleaning since those are the 
> > places that touch the directory mapping

I just looked at the LogManager and looks like they have their own internal 
directory mapping that uses File objects, and LogManager constructor only 
accepts array of File directories so I don't think relative/absolute or 
different path trailing conventions has much meaning to be tested for LogMgr. 
Thoughts?  


- Timothy


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


On March 25, 2014, 6:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19626/
> ---
> 
> (Updated March 25, 2014, 6:54 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1323
> https://issues.apache.org/jira/browse/KAFKA-1323
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1323 Fix log directory to support relative directories
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 0b88f14c4855b27242906cd45930bae501e26226 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 255be063ee247a849e527297c987da4625d749ca 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
> b5936d4101b513baa805ab26361fe965bdf980aa 
> 
> Diff: https://reviews.apache.org/r/19626/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 4:48 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 

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


Testing
---


Thanks,

Timothy Chen

Review Request 19690: Patch for KAFKA-1341

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

Fix selector connection id checking


Diffs
-

  clients/src/main/java/org/apache/kafka/common/network/Selector.java 
983963200ce81614577cd6182a5d2f10c22b95d4 
  clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
90e2dcf5434db546387302fb0219edfdb363592e 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 6:31 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
7dc27186dec23eccef934b0a1af9f320553e6c7c 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
488dfd08d9956dab2fb1ed3925d138cda637509d 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 6:59 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1317 Fix Kafka shutdown with delete topic on
This is based off of origin/trunk


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
7dc27186dec23eccef934b0a1af9f320553e6c7c 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
488dfd08d9956dab2fb1ed3925d138cda637509d 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 10:05 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1317 Fix Kafka shutdown with delete topic on
This is based on origin/0.8.1


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Help in setting up Kafka cluster

[Timothy Chen]

Hi Roy,

I wonder if you were able to start the broker following the steps here:

http://kafka.apache.org/documentation.html#quickstart

That page also shows you how to create a topic and send/consume messages
using the console producer/consumer.

Let us know if you run into any problems,

Tim


On Wed, Mar 26, 2014 at 3:04 PM, , Roy  wrote:

> Hi,
>
>   First time I am trying to setup new kafka cluster. I have tried sudo
> cluster with cli based kafka producer and consumer.
>
> Having difficulties with setting up log aggregation kafka producer and
> consumer.
>
> I would appreciate if anyone can help me in this.
>
> - roy
>

Re: Review Request 19577: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 10:09 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
f12ffc2af1f739f7d46058c97f0eeed6b55da14d 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
8262e10ce4364f057e449172ace38320e44247db 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 26, 2014, 10:19 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
7dc27186dec23eccef934b0a1af9f320553e6c7c 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
488dfd08d9956dab2fb1ed3925d138cda637509d 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

> On March 26, 2014, 12:55 a.m., Neha Narkhede wrote:
> > My guess is that we may have to review and make a similar change in the 
> > following places -
> > 1. LogManager.checkpointRecoveryPointOffsets()
> > 2. LogManager.nextLogDir()
> > 
> > The unit test in ReplicaManagerTest cover checkpointing, but we also need 
> > to add coverage for log recovery and log cleaning since those are the 
> > places that touch the directory mapping
> 
> Timothy Chen wrote:
> I just looked at the LogManager and looks like they have their own 
> internal directory mapping that uses File objects, and LogManager constructor 
> only accepts array of File directories so I don't think relative/absolute or 
> different path trailing conventions has much meaning to be tested for LogMgr. 
> Thoughts?
> 
> Neha Narkhede wrote:
> hmm.. checkpointRecoveryPointOffsets() also seemed to access the parent 
> directory -
> 
> val recoveryPointsByDir = 
> this.logsByTopicPartition.groupBy(_._2.dir.getParent.toString)
> 
> Same in nextLogDir() -
> 
>   val logCounts = allLogs.groupBy(_.dir.getParent).mapValues(_.size)
>   val zeros = logDirs.map(dir => (dir.getPath, 0)).toMap
> 
> I'm not sure that this will necessarily be a problem, but it will be good 
> to get unit test coverage for this bug for the above 2 APIs. In general, that 
> would be log recovery and log cleaning.

I think the difference really is that in ReplicaManagerTest we read in 
configuration log.dirs directly as string and store them in the map keys, where 
here they're all wrapped with File object so paths have the same convention 
when they are returned.

I will add Log manager tests just to verify this.


- Timothy


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


On March 25, 2014, 6:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19626/
> ---
> 
> (Updated March 25, 2014, 6:54 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1323
> https://issues.apache.org/jira/browse/KAFKA-1323
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1323 Fix log directory to support relative directories
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 0b88f14c4855b27242906cd45930bae501e26226 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 255be063ee247a849e527297c987da4625d749ca 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
> b5936d4101b513baa805ab26361fe965bdf980aa 
> 
> Diff: https://reviews.apache.org/r/19626/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

> On March 26, 2014, 10:59 p.m., Neha Narkhede wrote:
> > When I applied this patch to trunk, I started seeing unit test failures. In 
> > my last test run, kafka.log4j.KafkaLog4jAppenderTest fails. Before this, 
> > I've never seen unit test failures, so I wonder if this patch is causing 
> > the regression. Could you please double-check?

I see the test failure too now, which I thought it was odd to see it in a 
totally unrelated area. 
I grabbed latest trunk and ran the tests too and see the test failure as well, 
so I can confirm it's also happening in trunk.
Interestingly if running the tests individually they pass, so I think it has 
something to do with other tests affecting the outcome of this one.
I'll be looking into this.


- Timothy


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


On March 26, 2014, 10:19 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19696/
> ---
> 
> (Updated March 26, 2014, 10:19 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1317
> https://issues.apache.org/jira/browse/KAFKA-1317
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1317 Fix Kafka shutdown with delete topic on
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 7dc27186dec23eccef934b0a1af9f320553e6c7c 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> 488dfd08d9956dab2fb1ed3925d138cda637509d 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
> 20fe93e623319fd82236eb6364d7f80bf7a256aa 
> 
> Diff: https://reviews.apache.org/r/19696/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 27, 2014, 10:15 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
7dc27186dec23eccef934b0a1af9f320553e6c7c 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
488dfd08d9956dab2fb1ed3925d138cda637509d 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
67497dd042dfd4ad54875fe29200d67f59137264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

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

(Updated March 27, 2014, 11:17 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1323 Fix log directory to support relative directories


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
255be063ee247a849e527297c987da4625d749ca 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
b4bee33191ebc0b4ab5a5f82fa232cd3bda8fef8 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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

(Updated March 28, 2014, 4:34 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1317 Fix Kafka shutdown with delete topic on


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
7dc27186dec23eccef934b0a1af9f320553e6c7c 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
488dfd08d9956dab2fb1ed3925d138cda637509d 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
67497dd042dfd4ad54875fe29200d67f59137264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
20fe93e623319fd82236eb6364d7f80bf7a256aa 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19696: Patch for KAFKA-1317

[Timothy Chen]

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



core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/19696/#comment71296>

I think it wasn't atomic since it's assuming that the methods are called 
within controllerContext lock.

To be safe we can make it AtomicBoolean and I don't think it has much 
performance impact either way.


- Timothy Chen


On March 28, 2014, 4:34 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19696/
> ---
> 
> (Updated March 28, 2014, 4:34 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1317
> https://issues.apache.org/jira/browse/KAFKA-1317
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1317 Fix Kafka shutdown with delete topic on
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 7dc27186dec23eccef934b0a1af9f320553e6c7c 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> 488dfd08d9956dab2fb1ed3925d138cda637509d 
>   core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
> 67497dd042dfd4ad54875fe29200d67f59137264 
>   core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
> b5936d4101b513baa805ab26361fe965bdf980aa 
>   core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
> 20fe93e623319fd82236eb6364d7f80bf7a256aa 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 2054c25bbced6bd90c092a1974975732ad346146 
> 
> Diff: https://reviews.apache.org/r/19696/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19828: Patch for KAFKA-1350

[Timothy Chen]

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



core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/19828/#comment71406>

Should we remove these guards? From profiling it does seem the 
String.format can potentially be more expensive than we thought 


- Timothy Chen


On March 30, 2014, 6:29 a.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19828/
> ---
> 
> (Updated March 30, 2014, 6:29 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1350
> https://issues.apache.org/jira/browse/KAFKA-1350
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Converting the request logger to extend from the Logging trait
> 
> 
> Converting the state change logger to use the Logger trait
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 8ab8ab66bda1e13986df9b88d49199136833de97 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 2867ef147320c470050256a6b87615e589a7808e 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala 
> c3e8d05102c38045df48936655291bc0f61da84f 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
>   core/src/main/scala/kafka/network/RequestChannel.scala 
> a6ec970d72fc8adc56bc6e200d1ff5bbf0d2b748 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> ae2df2014a08aaa95b5eaa430684cfdb79d4f55e 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 0fe881d22ff8ada9f2596b118e0dbb6f8cd8471c 
> 
> Diff: https://reviews.apache.org/r/19828/diff/
> 
> 
> Testing
> ---
> 
> Locally tested by monitoring the state change & request log. 
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19828: Patch for KAFKA-1350

[Timothy Chen]

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



core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/19828/#comment71422>

Not sure if I'm missing something, but isn't the string format is going to 
be called here no matter what? I don't think this is a lambda function right?


- Timothy Chen


On March 30, 2014, 6:29 a.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19828/
> ---
> 
> (Updated March 30, 2014, 6:29 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1350
> https://issues.apache.org/jira/browse/KAFKA-1350
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Converting the request logger to extend from the Logging trait
> 
> 
> Converting the state change logger to use the Logger trait
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 8ab8ab66bda1e13986df9b88d49199136833de97 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 2867ef147320c470050256a6b87615e589a7808e 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala 
> c3e8d05102c38045df48936655291bc0f61da84f 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
>   core/src/main/scala/kafka/network/RequestChannel.scala 
> a6ec970d72fc8adc56bc6e200d1ff5bbf0d2b748 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> ae2df2014a08aaa95b5eaa430684cfdb79d4f55e 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 0fe881d22ff8ada9f2596b118e0dbb6f8cd8471c 
> 
> Diff: https://reviews.apache.org/r/19828/diff/
> 
> 
> Testing
> ---
> 
> Locally tested by monitoring the state change & request log. 
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19510: Patch for KAFKA-1318-v2

[Timothy Chen]

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


Not sure if I'm missing something here, but I don't see any Popen.communicate 
changes in this rb?

- Timothy Chen


On March 31, 2014, 5:32 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19510/
> ---
> 
> (Updated March 31, 2014, 5:32 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1318 and KAFKA-1318-v2
> https://issues.apache.org/jira/browse/KAFKA-1318
> https://issues.apache.org/jira/browse/KAFKA-1318-v2
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> 1. Used Popen.communicate to wait for producer to stop. 2. Added the 
> log4j.properties for migration tool testsuite. 3. Passed in the right log4j 
> property file for the broker. 4. Removed topic creating in migration tool 
> test script since it's trying to run the 0.8 topic creation script on the 0.7 
> broker. Instead, increased retry.backoff.ms in migration producer to avoid 
> data loss on auto created topic. 5. Improved README.
> 
> 
> Diffs
> -
> 
>   system_test/README.txt 87937ecd7d637c62f2d401f54f1d669ac0d13f0a 
>   system_test/migration_tool_testsuite/0.7/config/log4j.properties 
> PRE-CREATION 
>   system_test/migration_tool_testsuite/config/migration_producer.properties 
> 17b5928a1213fc0cae7cd94cd214323ccca61d48 
>   system_test/migration_tool_testsuite/migration_tool_test.py 
> 2386a5884fcf86bac282ef28f07a3af740185f7f 
>   system_test/utils/kafka_system_test_utils.py 
> 35f2d1b0464edb738cd8e01f8654913776c49459 
> 
> Diff: https://reviews.apache.org/r/19510/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 19510: Patch for KAFKA-1318-v2

[Timothy Chen]

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


Ah found it! Was searching for popen earlier.

- Timothy Chen


On March 31, 2014, 5:32 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19510/
> ---
> 
> (Updated March 31, 2014, 5:32 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1318 and KAFKA-1318-v2
> https://issues.apache.org/jira/browse/KAFKA-1318
> https://issues.apache.org/jira/browse/KAFKA-1318-v2
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> 1. Used Popen.communicate to wait for producer to stop. 2. Added the 
> log4j.properties for migration tool testsuite. 3. Passed in the right log4j 
> property file for the broker. 4. Removed topic creating in migration tool 
> test script since it's trying to run the 0.8 topic creation script on the 0.7 
> broker. Instead, increased retry.backoff.ms in migration producer to avoid 
> data loss on auto created topic. 5. Improved README.
> 
> 
> Diffs
> -
> 
>   system_test/README.txt 87937ecd7d637c62f2d401f54f1d669ac0d13f0a 
>   system_test/migration_tool_testsuite/0.7/config/log4j.properties 
> PRE-CREATION 
>   system_test/migration_tool_testsuite/config/migration_producer.properties 
> 17b5928a1213fc0cae7cd94cd214323ccca61d48 
>   system_test/migration_tool_testsuite/migration_tool_test.py 
> 2386a5884fcf86bac282ef28f07a3af740185f7f 
>   system_test/utils/kafka_system_test_utils.py 
> 35f2d1b0464edb738cd8e01f8654913776c49459 
> 
> Diff: https://reviews.apache.org/r/19510/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

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

(Updated March 31, 2014, 6:57 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1323 Fix log directory to support relative directories


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
255be063ee247a849e527297c987da4625d749ca 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
b4bee33191ebc0b4ab5a5f82fa232cd3bda8fef8 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
67497dd042dfd4ad54875fe29200d67f59137264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19690: Patch for KAFKA-1341

[Timothy Chen]

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

(Updated March 31, 2014, 9:42 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1341 Client Selector doesn't check connection id properly


KAFKA-1323 Fix log directory to support relative directories


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/network/Selector.java 
5d93965a33bedc471fe6126277992591e0560add 
  clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
99856e93f307e5e3db045653355bfb0d19114a0d 
  core/src/main/scala/kafka/cluster/Partition.scala 
0b88f14c4855b27242906cd45930bae501e26226 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
255be063ee247a849e527297c987da4625d749ca 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
b4bee33191ebc0b4ab5a5f82fa232cd3bda8fef8 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
67497dd042dfd4ad54875fe29200d67f59137264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19690: Patch for KAFKA-1341

[Timothy Chen]

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

(Updated March 31, 2014, 9:44 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1341 Client Selector doesn't check connection id properly


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/network/Selector.java 
5d93965a33bedc471fe6126277992591e0560add 
  clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
99856e93f307e5e3db045653355bfb0d19114a0d 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19828: Patch for KAFKA-1350

[Timothy Chen]

> On March 31, 2014, 1:40 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 141
> > <https://reviews.apache.org/r/19828/diff/2/?file=540411#file540411line141>
> >
> > I see what you are saying now. Yes, I think you are right. This patch 
> > only addresses the conversion to the standard logging trait but the string 
> > formatting will still happen. 
> > 
> > I think this JIRA is filed to fix the actual logging due to the absence 
> > of the check. I've filed KAFKA-1351 to fix all the String.format uses 
> > properly, which is a general problem much beyond just the state change 
> > logger.
> 
> Guozhang Wang wrote:
> Actually if we use the Logging trait the format function would not be 
> called if we are not on that logging level. I did a small test and comment on 
> the jira.

Yes you are correct, this shouldn't be a issue if we use our logging trait.


- Timothy


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


On March 30, 2014, 6:29 a.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19828/
> ---
> 
> (Updated March 30, 2014, 6:29 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1350
> https://issues.apache.org/jira/browse/KAFKA-1350
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Converting the request logger to extend from the Logging trait
> 
> 
> Converting the state change logger to use the Logger trait
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 8ab8ab66bda1e13986df9b88d49199136833de97 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 2867ef147320c470050256a6b87615e589a7808e 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala 
> c3e8d05102c38045df48936655291bc0f61da84f 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
>   core/src/main/scala/kafka/network/RequestChannel.scala 
> a6ec970d72fc8adc56bc6e200d1ff5bbf0d2b748 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> ae2df2014a08aaa95b5eaa430684cfdb79d4f55e 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 0fe881d22ff8ada9f2596b118e0dbb6f8cd8471c 
> 
> Diff: https://reviews.apache.org/r/19828/diff/
> 
> 
> Testing
> ---
> 
> Locally tested by monitoring the state change & request log. 
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19828: Patch for KAFKA-1350

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On March 30, 2014, 6:29 a.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19828/
> ---
> 
> (Updated March 30, 2014, 6:29 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1350
> https://issues.apache.org/jira/browse/KAFKA-1350
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Converting the request logger to extend from the Logging trait
> 
> 
> Converting the state change logger to use the Logger trait
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 8ab8ab66bda1e13986df9b88d49199136833de97 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 2867ef147320c470050256a6b87615e589a7808e 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala 
> c3e8d05102c38045df48936655291bc0f61da84f 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
>   core/src/main/scala/kafka/network/RequestChannel.scala 
> a6ec970d72fc8adc56bc6e200d1ff5bbf0d2b748 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> ae2df2014a08aaa95b5eaa430684cfdb79d4f55e 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 0fe881d22ff8ada9f2596b118e0dbb6f8cd8471c 
> 
> Diff: https://reviews.apache.org/r/19828/diff/
> 
> 
> Testing
> ---
> 
> Locally tested by monitoring the state change & request log. 
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19879: Further metric name standardization

[Timothy Chen]

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



clients/src/main/java/org/apache/kafka/common/network/Selector.java
<https://reviews.apache.org/r/19879/#comment71554>

I'm not sure what our naming convention is, but one thing I notice is that 
we sometimes add ns (nanoseconds) in the name and sometimes doesn't. Do we want 
to have a convention for units or?


- Timothy Chen


On April 1, 2014, 7:49 p.m., Jay Kreps wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19879/
> ---
> 
> (Updated April 1, 2014, 7:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1251
> https://issues.apache.org/jira/browse/KAFKA-1251
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1251 Further metric naming standardization.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 8c1c5751a9186e6bc3196d152e4c3e948cc34b39 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> c92bfbcd02b5444d2673b00f9699a553a40a877a 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 5b801e47faab3d365b771396fb6e9f7f66600109 
> 
> Diff: https://reviews.apache.org/r/19879/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>

Re: Review Request 19879: Further metric name standardization

[Timothy Chen]

> On April 1, 2014, 8:21 p.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 
> > 432
> > <https://reviews.apache.org/r/19879/diff/1/?file=544095#file544095line432>
> >
> > Here was my convention:
> > 1. All rates are in seconds (the default)
> > 2. All latencies are in ms unless otherwise noted.
> > 3. A few things require finer grained measurement than 1 ms and for 
> > these I called it out in the name so this would not be confusing.

Then perhaps io-ratio needs to add a ns in it as it's in nanoseconds right?


- Timothy


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


On April 1, 2014, 7:49 p.m., Jay Kreps wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19879/
> ---
> 
> (Updated April 1, 2014, 7:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1251
> https://issues.apache.org/jira/browse/KAFKA-1251
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1251 Further metric naming standardization.
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 8c1c5751a9186e6bc3196d152e4c3e948cc34b39 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> c92bfbcd02b5444d2673b00f9699a553a40a877a 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 5b801e47faab3d365b771396fb6e9f7f66600109 
> 
> Diff: https://reviews.apache.org/r/19879/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>

Re: Review Request 19626: Patch for KAFKA-1323

[Timothy Chen]

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

(Updated April 2, 2014, 7:18 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1323 Fix log directory to support relative directories


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
6d6b4ab91608162201d2737d784580728193566a 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
0f5aaa9888d6c0ca0b073f4742bee14ad72d0484 
  core/src/test/scala/unit/kafka/log/LogManagerTest.scala 
b4bee33191ebc0b4ab5a5f82fa232cd3bda8fef8 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
67497dd042dfd4ad54875fe29200d67f59137264 
  core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala 
b5936d4101b513baa805ab26361fe965bdf980aa 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19953: Patch for KAFKA-1303

[Timothy Chen]

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



clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/19953/#comment71700>

I wonder if it's possible to add a test for this?



clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
<https://reviews.apache.org/r/19953/#comment71702>

The node selection doesn't seem to be random though, it seems like it's 
going to favor the lower broker nodes first?


- Timothy Chen


On April 2, 2014, 6:49 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19953/
> ---
> 
> (Updated April 2, 2014, 6:49 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1303
> https://issues.apache.org/jira/browse/KAFKA-1303
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Changes are described in the comment of selectMetadataDestination().
> 
> 
> 1. Added the logic to use bootstrap cluster if no node can be selected from 
> the real cluster. 2. Fixed the issue that node is not selected randomally.
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 
> 855ae84f14aa91653b3fa855c2af40560323f42a 
> 
> Diff: https://reviews.apache.org/r/19953/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/server/KafkaApis.scala 
c068ef69207c351eec413a595f1747c59f8b3983 

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


Testing
---


Thanks,

Timothy Chen

Review Request 19972: Patch for KAFKA-1358

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1358 Only shutdown delete topic manager if initialized on reinit zk 
session


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
d142f8c1859bfb8660a399dca16ffebdc30d8365 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 3, 2014, 1:13 a.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 638
> > <https://reviews.apache.org/r/19957/diff/1/?file=546094#file546094line638>
> >
> > I think this really should have been 
> > isr.map(aliveBrokers.get).filter(_.isEmpty)

this is passed to a function that only accepts Seq[Broker] and even if you 
filter the return type still is a Seq[Option[Broker]].
I think I'll just leave it as is


- Timothy


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


On April 2, 2014, 8:44 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 2, 2014, 8:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 3, 2014, 1:13 a.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/api/TopicMetadata.scala, line 35
> > <https://reviews.apache.org/r/19957/diff/1/?file=546093#file546093line35>
> >
> > Nitpick, but I think this would be a little neater:
> > (0 to numPartitions).map(  ).toArray

(0 to numPartitions).map(null).toArray actually gives me a null pointer 
exception :)


- Timothy


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


On April 2, 2014, 8:44 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 2, 2014, 8:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 3, 2014, 1:39 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs (updated)
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
  core/src/main/scala/kafka/server/KafkaApis.scala 
c068ef69207c351eec413a595f1747c59f8b3983 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote:
> > Just curious: which part of the code is the main contribution to the 
> > handling overhead?
> 
> Joel Koshy wrote:
> Tim can correct me if I'm wrong but I think the overhead was largely in 
> the sort.
> 
> Thanks again for the patch. I'll get this checked in.

>From profiling with 1000 topics actually the largest contribution looks like 
>was the metadataCache.map(_.topic) call, which originally we call for each 
>topic that is being requested. I moved it so that we just do it once. 


- Timothy


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


On April 3, 2014, 1:39 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 3, 2014, 1:39 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote:
> > Just curious: which part of the code is the main contribution to the 
> > handling overhead?
> 
> Joel Koshy wrote:
> Tim can correct me if I'm wrong but I think the overhead was largely in 
> the sort.
> 
> Thanks again for the patch. I'll get this checked in.
> 
> Timothy Chen wrote:
> From profiling with 1000 topics actually the largest contribution looks 
> like was the metadataCache.map(_.topic) call, which originally we call for 
> each topic that is being requested. I moved it so that we just do it once.

Sorry left out an important detail, the most time was spent in HashSet.updated0 
call which looks like it's from metadataCache.map(_.topic).toSet. The 
construction of the new set seems to be expensive and we do it for each entry 
of topic and partition.


- Timothy


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


On April 3, 2014, 1:39 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 3, 2014, 1:39 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 4, 2014, 3:59 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, lines 619-620
> > <https://reviews.apache.org/r/19957/diff/2/?file=547050#file547050line619>
> >
> > If this is expensive, perhaps we can only do that if we require all 
> > topics? If that makes the code too complicated, we don't have to do this 
> > optimization.
> 
> Neha Narkhede wrote:
> Since this is invoked just once, it's not worth optimizing. Is it?
> 
> Jun Rao wrote:
> We don't have to if it's too complicated. However, most requests will 
> have a small # of topics in the request. In those cases, there is no need to 
> make a copy of all cached topics.

The original code actually does a toSet of all the topics in each topic 
requested.
I just refactored metadata cache so it's organized by topic.


- Timothy


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


On April 3, 2014, 1:39 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 3, 2014, 1:39 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 4, 2014, 9:40 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs (updated)
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
  core/src/main/scala/kafka/server/KafkaApis.scala 
c068ef69207c351eec413a595f1747c59f8b3983 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
22bb6f2847b8895f8fbba6c531963ebb0fffe2ca 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20038: Patch for KAFKA-1355

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 4, 2014, 8:51 p.m., Joel Koshy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> ---
> 
> (Updated April 4, 2014, 8:51 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Avoid sending all topic metadata on state changes.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 
> 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> f17d97603798e803d035f313f0165f11f6d0f2c0 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> 09f54acc733d49a9be9a9c6633271c190dea1bf0 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
> 
> Diff: https://reviews.apache.org/r/20038/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joel Koshy
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

> On April 5, 2014, 12:11 a.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/api/TopicMetadata.scala, line 35
> > <https://reviews.apache.org/r/19957/diff/3/?file=549380#file549380line35>
> >
> > could you confirm why we need to fill the array vs create an empty one 
> > of size numPartitions?

I realized I was using scala's immutable array when I was trying it, with a 
mutable array it works. Fixing it now.


- Timothy


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


On April 4, 2014, 9:40 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19957/
> ---
> 
> (Updated April 4, 2014, 9:40 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356 Improve topic metadata handling in kafka api
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/api/TopicMetadata.scala 
> 0513a59ed94e556894b3515dc38666ee9a66ae3d 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> c068ef69207c351eec413a595f1747c59f8b3983 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 
> d5644ea40ec7678b975c4775546b79fcfa9f64b7 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
> 22bb6f2847b8895f8fbba6c531963ebb0fffe2ca 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 2054c25bbced6bd90c092a1974975732ad346146 
> 
> Diff: https://reviews.apache.org/r/19957/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 5, 2014, 12:45 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs (updated)
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
  core/src/main/scala/kafka/server/KafkaApis.scala 
c068ef69207c351eec413a595f1747c59f8b3983 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
22bb6f2847b8895f8fbba6c531963ebb0fffe2ca 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 6, 2014, 8:46 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs (updated)
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
fcabd0da201ccd478f5884bcca9b75a8c8ea8373 
  core/src/main/scala/kafka/server/KafkaApis.scala 
c068ef69207c351eec413a595f1747c59f8b3983 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
22bb6f2847b8895f8fbba6c531963ebb0fffe2ca 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 19731: Patch for KAFKA-1328

[Timothy Chen]

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



clients/src/main/java/kafka/clients/consumer/Consumer.java
<https://reviews.apache.org/r/19731/#comment72276>

I wonder if it's worth adding more comments about the poll behavior here, 
about what conditions will it return from poll (ie: when timeout hit, first 
topic data available, etc).



clients/src/main/java/kafka/clients/consumer/Consumer.java
<https://reviews.apache.org/r/19731/#comment72303>

I know we return custom future objects for the producer, is there a reason 
we don't keep it consistent on the consumer?



clients/src/main/java/kafka/clients/consumer/ConsumerConfig.java
<https://reviews.apache.org/r/19731/#comment72277>

What happens when you explicitly call commit when auto commit is enabled?



clients/src/main/java/kafka/clients/consumer/ConsumerConfig.java
<https://reviews.apache.org/r/19731/#comment72278>

Don't we throw an exception instead of being stuck? Being stuck doesn't 
sounds like a acceptable behavior



clients/src/main/java/kafka/clients/consumer/KafkaConsumer.java
<https://reviews.apache.org/r/19731/#comment72305>

How does one actually get the list of partitions that returns failure?



clients/src/main/java/kafka/clients/consumer/KafkaConsumer.java
<https://reviews.apache.org/r/19731/#comment72279>

Potentially in this sample code you could throw an exception in your catch 
block and never call close. Is that ok too?



clients/src/main/java/kafka/clients/consumer/MockConsumer.java
<https://reviews.apache.org/r/19731/#comment72313>

Looks like the mock consumer doesn't follow the mutually exclusive rule.


- Timothy Chen


On March 27, 2014, 4:16 p.m., Neha Narkhede wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19731/
> ---
> 
> (Updated March 27, 2014, 4:16 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1328
> https://issues.apache.org/jira/browse/KAFKA-1328
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Added license headers
> 
> 
> Cleaned javadoc for ConsumerConfig
> 
> 
> Fixed minor indentation in ConsumerConfig
> 
> 
> Improve docs on ConsumerConfig
> 
> 
> 1. Added ClientUtils 2. Added basic constructor implementation for 
> KafkaConsumer
> 
> 
> Improved MockConsumer
> 
> 
> Chris's feedback and also consumer rewind example code
> 
> 
> Added commit() and commitAsync() APIs to the consumer and updated docs and 
> examples to reflect that
> 
> 
> 1. Added consumer usage examples to javadoc 2. Changed signature of APIs that 
> accept or return offsets from list of offsets to map of offsets
> 
> 
> Improved example for using ConsumerRebalanceCallback
> 
> 
> Improved example for using ConsumerRebalanceCallback
> 
> 
> Included Jun's review comments and renamed positions to seek. Also included 
> position()
> 
> 
> Changes to javadoc for positions()
> 
> 
> Changed the javadoc for ConsumerRebalanceCallback
> 
> 
> Changing unsubscribe to also take in var args for topic list
> 
> 
> Incorporated first round of feedback from Jay, Pradeep and Mattijs on the 
> mailing list
> 
> 
> Updated configs
> 
> 
> Javadoc for consumer complete
> 
> 
> Completed docs for Consumer and ConsumerRebalanceCallback. Added MockConsumer
> 
> 
> Added the initial interfaces and related documentation for the consumer. More 
> docs required to complete the public API
> 
> 
> Diffs
> -
> 
>   clients/src/main/java/kafka/clients/consumer/Consumer.java PRE-CREATION 
>   clients/src/main/java/kafka/clients/consumer/ConsumerConfig.java 
> PRE-CREATION 
>   clients/src/main/java/kafka/clients/consumer/ConsumerRebalanceCallback.java 
> PRE-CREATION 
>   clients/src/main/java/kafka/clients/consumer/ConsumerRecord.java 
> PRE-CREATION 
>   clients/src/main/java/kafka/clients/consumer/KafkaConsumer.java 
> PRE-CREATION 
>   clients/src/main/java/kafka/clients/consumer/MockConsumer.java PRE-CREATION 
>   clients/src/main/java/kafka/common/TopicPartitionOffset.java PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 1ff9174870a8c9cd97eb6655416edd4124377b0e 
>   clients/src/main/java/org/apache/kafka/common/utils/ClientUtils.java 
> PRE-CREATION 
>   
> clients/src/test/java/org/apache/kafka/clients/consumer/ConsumerExampleTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19731/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neha Narkhede
> 
>

Re: Review Request 19957: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 8, 2014, 8:38 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1356 Improve topic metadata handling in kafka api


Diffs (updated)
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
c8c02ced543a6278ba5b1edfa73a370f1edb1891 
  core/src/main/scala/kafka/server/KafkaApis.scala 
705d87ed874f2363e6d9d6ea1143bc6035a0a9d6 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
22bb6f2847b8895f8fbba6c531963ebb0fffe2ca 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
2054c25bbced6bd90c092a1974975732ad346146 

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


Testing
---


Thanks,

Timothy Chen

Review Request 20143: Patch for KAFKA-1363

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1363 Avoid delete topic deadlock by checking thread run status


Diffs
-

  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
d29e556232ae549545bde1b6c5c9fabd5fa67bf5 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20143: Patch for KAFKA-1363

[Timothy Chen]

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

(Updated April 9, 2014, 12:52 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1363 Avoid delete topic deadlock by checking thread run status


Diffs (updated)
-

  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
d29e556232ae549545bde1b6c5c9fabd5fa67bf5 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20130: Patch for KAFKA-1373

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 8, 2014, 11:18 p.m., Joel Koshy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20130/
> ---
> 
> (Updated April 8, 2014, 11:18 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1373
> https://issues.apache.org/jira/browse/KAFKA-1373
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Set first dirty (uncompacted) offset to first offset of the log if no 
> checkpoint exists.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/log/LogCleanerManager.scala 
> 6a981345fd7dc711e6c4f058a42bd7f8f9d350fe 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> b0506d4881176cd95914847c4275e428a8d2ca10 
>   core/src/main/scala/kafka/server/OffsetCheckpoint.scala 
> 19f61a9718a7f8e6f9bf743ac9eb2118d0fdd9b0 
>   core/src/main/scala/kafka/server/OffsetManager.scala 
> 89a88a7e4055f804429b64e85b2f65312d1e2155 
>   core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 
> 9aeb69d4934b9aa94e0899af14fc27e7c20f039f 
> 
> Diff: https://reviews.apache.org/r/20130/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joel Koshy
> 
>

Re: Review Request 20143: Patch for KAFKA-1363

[Timothy Chen]

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

(Updated April 9, 2014, 6:38 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1363 Avoid delete topic deadlock by checking thread run status


Diffs (updated)
-

  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
d29e556232ae549545bde1b6c5c9fabd5fa67bf5 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20186: Patch for KAFKA-1377

[Timothy Chen]

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



core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
<https://reviews.apache.org/r/20186/#comment72754>

I thought we want to assert here so we know from the output it's the leader 
not being elected being the problem in test?


- Timothy Chen


On April 10, 2014, 1:22 a.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20186/
> ---
> 
> (Updated April 10, 2014, 1:22 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1377
> https://issues.apache.org/jira/browse/KAFKA-1377
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Increase the wait timeout to give the leader a better chance of being elected.
> 
> 
> Diffs
> -
> 
>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala 
> e1022850d1efc828b8135fb0aa3f19801afc 
> 
> Diff: https://reviews.apache.org/r/20186/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Review Request 20187: Patch for KAFKA-1363

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1363 Avoid delete topic deadlock by checking thread run status


Diffs
-

  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
40c4c5776b893890a30ad1e4b04f36aa1c112d80 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20187: Patch for KAFKA-1363

[Timothy Chen]

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

(Updated April 10, 2014, 1:27 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1363 Avoid delete topic deadlock by checking thread run status


Diffs
-

  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
40c4c5776b893890a30ad1e4b04f36aa1c112d80 

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


Testing
---


Thanks,

Timothy Chen

Review Request 20190: Patch for KAFKA-1356

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

Refactor metadata cache code in kafka api and remove lock on ensureTopicExists


Diffs
-

  core/src/main/scala/kafka/server/KafkaApis.scala 
d96229e2d4aa7006b0dbd81055ce5a2459d8758c 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20190: Patch for KAFKA-1356

[Timothy Chen]

> On April 10, 2014, 2:57 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, lines 62-63
> > <https://reviews.apache.org/r/20190/diff/1/?file=554248#file554248line62>
> >
> > Could we make this class private?

Unit tests seems to rely on accessing the cache to verify some behaviors, so I 
thought to leave it open as is.


> On April 10, 2014, 2:57 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, lines 283-284
> > <https://reviews.apache.org/r/20190/diff/1/?file=554248#file554248line283>
> >
> > I am not sure that we should just blindly set the controllerEpoch in 
> > ReplicaManager. ReplicaManager is supposed to hold the latest controller 
> > epoch so that it can disregard requests sent from an older controller. So, 
> > we only need to update controllerEpoch if the new value is higher than the 
> > current one. Perhaps we can add a util function in ReplicaManager to do 
> > that.

Talked to Jun and we agree the previous check is sufficient here.


- Timothy


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


On April 10, 2014, 1:48 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20190/
> ---
> 
> (Updated April 10, 2014, 1:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1356
> https://issues.apache.org/jira/browse/KAFKA-1356
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Refactor metadata cache code in kafka api and remove lock on ensureTopicExists
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> d96229e2d4aa7006b0dbd81055ce5a2459d8758c 
> 
> Diff: https://reviews.apache.org/r/20190/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 20130: Patch for KAFKA-1373

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 8, 2014, 11:18 p.m., Joel Koshy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20130/
> ---
> 
> (Updated April 8, 2014, 11:18 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1373
> https://issues.apache.org/jira/browse/KAFKA-1373
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Set first dirty (uncompacted) offset to first offset of the log if no 
> checkpoint exists.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/log/LogCleanerManager.scala 
> 6a981345fd7dc711e6c4f058a42bd7f8f9d350fe 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> b0506d4881176cd95914847c4275e428a8d2ca10 
>   core/src/main/scala/kafka/server/OffsetCheckpoint.scala 
> 19f61a9718a7f8e6f9bf743ac9eb2118d0fdd9b0 
>   core/src/main/scala/kafka/server/OffsetManager.scala 
> 89a88a7e4055f804429b64e85b2f65312d1e2155 
>   core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 
> 9aeb69d4934b9aa94e0899af14fc27e7c20f039f 
> 
> Diff: https://reviews.apache.org/r/20130/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joel Koshy
> 
>

Review Request 20252: Patch for KAFKA-1356

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

WIP


Diffs
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
d6c03218ec0afdb65a7b5aa9b85eb1c029357d7a 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0f137c5136e2320ca27c285d6ab013f6559314c4 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
1317b4c3c60b8d1835dd6a06bf9b250398f0d47d 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
500eeca2f95d901536b1363b8c4b485c4893179f 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20252: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 11, 2014, 7:41 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

WIP


Diffs
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
d6c03218ec0afdb65a7b5aa9b85eb1c029357d7a 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0f137c5136e2320ca27c285d6ab013f6559314c4 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
1317b4c3c60b8d1835dd6a06bf9b250398f0d47d 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
500eeca2f95d901536b1363b8c4b485c4893179f 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20252: Patch for KAFKA-1356

[Timothy Chen]

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

(Updated April 11, 2014, 7:42 a.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

Topic metadata requests takes too long to process


Diffs
-

  core/src/main/scala/kafka/api/TopicMetadata.scala 
0513a59ed94e556894b3515dc38666ee9a66ae3d 
  core/src/main/scala/kafka/controller/KafkaController.scala 
d6c03218ec0afdb65a7b5aa9b85eb1c029357d7a 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0f137c5136e2320ca27c285d6ab013f6559314c4 
  core/src/test/scala/unit/kafka/admin/AdminTest.scala 
d5644ea40ec7678b975c4775546b79fcfa9f64b7 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
1317b4c3c60b8d1835dd6a06bf9b250398f0d47d 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
500eeca2f95d901536b1363b8c4b485c4893179f 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20536: Patch for KAFKA-1380

[Timothy Chen]

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



core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/20536/#comment74283>

Isn't this a unnecessary check? The parent if check already checks the flag.


- Timothy Chen


On April 21, 2014, 9:34 p.m., Joel Koshy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20536/
> ---
> 
> (Updated April 21, 2014, 9:34 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356; follow-up - return unknown topic partition on non-existent topic 
> if auto.create is off
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 9e569eb8c3586b2fdb3b5904c5fa5f938cffada1 
> 
> Diff: https://reviews.apache.org/r/20536/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joel Koshy
> 
>

Re: Review Request 20534: Patch for KAFKA-1380

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 21, 2014, 9:47 p.m., Joel Koshy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20534/
> ---
> 
> (Updated April 21, 2014, 9:47 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1356; follow-up - return unknown topic partition on non-existent topic 
> if auto.create is off
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 9e569eb8c3586b2fdb3b5904c5fa5f938cffada1 
> 
> Diff: https://reviews.apache.org/r/20534/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joel Koshy
> 
>

Re: Review Request 20540: Patch for KAFKA-1410

[Timothy Chen]

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



core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/20540/#comment74401>

Do we no longer need to check for offset management topic?


- Timothy Chen


On April 22, 2014, 4:37 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20540/
> ---
> 
> (Updated April 22, 2014, 4:37 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1410
> https://issues.apache.org/jira/browse/KAFKA-1410
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> 1. Move MetadataCache to its own class. 2. Avoid updating metadata cache if 
> the controller epoch is old.
> 
> 
> 1. Remove ensureTopicExists(). 2. Move the update of controller epoc in topic 
> metadata request to ReplicaManager.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 1a4ffcea37e32739b8360956b4a40c0101b97822 
>   core/src/main/scala/kafka/server/MetadataCache.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 5588f59783d5cd8f074263c207045625282af12d 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
> 17b08e14f5e9bfec1233c80d74b50bc2ec38aa25 
> 
> Diff: https://reviews.apache.org/r/20540/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: Review Request 20540: Patch for KAFKA-1410

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 22, 2014, 8:51 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20540/
> ---
> 
> (Updated April 22, 2014, 8:51 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1410
> https://issues.apache.org/jira/browse/KAFKA-1410
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> 1. Handle the creation of OffsetTopic properly.
> 
> 
> 1. Move MetadataCache to its own class. 2. Avoid updating metadata cache if 
> the controller epoch is old.
> 
> 
> 1. Remove ensureTopicExists(). 2. Move the update of controller epoc in topic 
> metadata request to ReplicaManager.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 1a4ffcea37e32739b8360956b4a40c0101b97822 
>   core/src/main/scala/kafka/server/MetadataCache.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 5588f59783d5cd8f074263c207045625282af12d 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
> 17b08e14f5e9bfec1233c80d74b50bc2ec38aa25 
> 
> Diff: https://reviews.apache.org/r/20540/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Re: [VOTE] Apache Kafka 0.8.1.1 Release Candidate 1

[Timothy Chen]

+1 (Non-binding).

Tim

On Thu, Apr 24, 2014 at 5:32 PM, Joel Koshy  wrote:
> +1
>
> One (non-critical) issue is that I think our packaging isn't complete
> for kafka-perf (we are only building for 2.8.0).
>
> On Tue, Apr 22, 2014 at 07:18:38PM -0400, Joe Stein wrote:
>> This is the first candidate for release of Apache Kafka 0.8.1.1
>>
>> Release Notes for the 0.8.1.1 release
>> https://people.apache.org/~joestein/kafka-0.8.1.1-candidate1/RELEASE_NOTES.html
>>
>> *** Please download, test and vote by Friday, April 25th, 4pm PT
>>
>> Kafka's KEYS file containing PGP keys we use to sign the release:
>> https://svn.apache.org/repos/asf/kafka/KEYS in addition to the md5, sha1
>> and sha2 (SHA256) checksum.
>>
>> * Release artifacts to be voted upon (source and binary):
>> https://people.apache.org/~joestein/kafka-0.8.1.1-candidate1/
>>
>> * Maven artifacts to be voted upon prior to release:
>> https://repository.apache.org/content/groups/staging/
>>
>> * The tag to be voted upon (off the 0.8.1 branch) is the 0.8.1.1 tag
>> https://git-wip-us.apache.org/repos/asf?p=kafka.git;a=tag;h=34237371b662f617dd80c75cd6282f97e047ef56
>>
>>
>> /***
>>  Joe Stein
>>  Founder, Principal Consultant
>>  Big Data Open Source Security LLC
>>  http://www.stealth.ly
>>  Twitter: @allthingshadoop 
>> /
>

Re: Review Request 20713: Patch for KAFKA-1424

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On April 25, 2014, 2:54 p.m., Jun Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20713/
> ---
> 
> (Updated April 25, 2014, 2:54 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1424
> https://issues.apache.org/jira/browse/KAFKA-1424
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Wait metadata is propagated before fetching.
> 
> 
> Diffs
> -
> 
>   core/src/test/scala/unit/kafka/producer/ProducerTest.scala 
> dc6a5ea4abcdff370f3b61c774bbde4e4157f412 
> 
> Diff: https://reviews.apache.org/r/20713/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jun Rao
> 
>

Review Request 20718: Patch for KAFKA-1384

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1384: Logging kafka state metric


Diffs
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/log/Log.scala 
46df8d99d977a3b010a9b9f4698187fa9bfb2498 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 
acda52b801714bcc182edc0ced925f0e4b493fc1 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20718: Patch for KAFKA-1384

[Timothy Chen]

> On April 25, 2014, 6:35 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/KafkaServerStartable.scala, line 55
> > <https://reviews.apache.org/r/20718/diff/1/?file=568568#file568568line55>
> >
> > Unused

This is actually exposed setting custom state that is not one of the defined 
enums. One example is for our internal custom kafka server startable override 
can allow more states that is not defined in BrokerStates. The alternative is 
to just come up with all possible enums and let that be the only option 
available, but I was thinking that might be a bit too limiting. I'm not sure if 
this approach is too permissive either, you have more thoughts?


> On April 25, 2014, 6:35 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/BrokerStates.scala, line 28
> > <https://reviews.apache.org/r/20718/diff/1/?file=568566#file568566line28>
> >
> > Just a thought: I'm wondering if it is better to have a bit-vector 
> > approach for states; although that will limit the number of possible states 
> > - but I think that is fine.
> > 
> > The main reason for this is that it will enable composing simultaneous 
> > states. E.g., in this approach you cannot distinguish state 5 from state 3 
> > (if a shutting down broker is the controller). Although we can probably 
> > infer that from the fact that the other brokers are likely in state (2) or 
> > by looking at the active controller count separately. Still, we currently 
> > allow more than one broker to shut down.
> > 
> > It also _might_ help catch erroneous dual states (due to bugs).
> > 
> > What you have is probably fine for lifecycle states (except for the 
> > above caveat). However, if we ever want to allow more-than-lifecycle states 
> > (e.g., under-replicated is a state we _might_ want to include on this - 
> > even though we have a separate URP mbean and it's not a lifecycle state; 
> > another example is "loading consumer offsets").

Erroneous dual states is something I thought of, especially when state changes 
rapidly it will be harder to detect.
The down side of having a bit field is also that you need to then also have to 
unset that state when you're going into another lifecycle state.

I think coming back to the high level of what we want for broker states, since 
really a broker should really just be in a state at one point of time, setting 
a particular state I think makes more sense.

Composite states probably makes more sense for different component states. 


- Timothy


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


On April 25, 2014, 5:25 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20718/
> ---
> 
> (Updated April 25, 2014, 5:25 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1384
> https://issues.apache.org/jira/browse/KAFKA-1384
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1384: Logging kafka state metric
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/log/Log.scala 
> 46df8d99d977a3b010a9b9f4698187fa9bfb2498 
>   core/src/main/scala/kafka/log/LogManager.scala 
> ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 
> acda52b801714bcc182edc0ced925f0e4b493fc1 
> 
> Diff: https://reviews.apache.org/r/20718/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 20718: Patch for KAFKA-1384

[Timothy Chen]

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

(Updated April 26, 2014, 7:09 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1384: Logging kafka state metric


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/log/Log.scala 
f20c232e2daa63a10d91b965af52801af656477c 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 
acda52b801714bcc182edc0ced925f0e4b493fc1 

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


Testing
---


Thanks,

Timothy Chen

Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

Review request for kafka.


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


Repository: kafka


Description
---

Fix delete topic tests and deadlock


Diffs
-

  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0b668f230c8556fdf08654ce522a11847d0bf39b 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

> On April 28, 2014, 7:43 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line 
> > 210
> > <https://reviews.apache.org/r/20745/diff/1/?file=569160#file569160line210>
> >
> > better to not leak the logic from delete topic manager here. It is 
> > worth exposing the check as an API in DeleteTopicManager. In any case, can 
> > you explain the motivation behind your change?

Sorry forgot to add more comments to the rb, have been chatting mostly with Jun 
about my fixes.
One test that was failing in delete topic test was a test that was handling 
request while topic being deleted, and it made sure it can never finish by 
shutting down one of the follower broker. Sending a fetch request to a topic 
being deleted returns a NotLeaderForPartition error code instead of expected 
UnknownTopicOrPartition.
 
The cause of this is that as part of the metadata cache changes in Kafka api, 
we removed the check if a topic exists in the cache when serving fetching 
request and now it tries to read message set from the replica manager, and even 
though a stop replica request with delete partition is already been sent and 
processed, the partition was somehow still in replica manager and it returns 
NotLeaderForPartition exception since there is no leader for this topic.

I found out that the partition was being recreated in the replica manager, 
because in the process of retrying deleting the same topic it change the failed 
replica state into OfflineReplica again that triggers a LeaderAndIsrRequest to 
all brokers for shrinking the Isr for that replica. 

The brokers that received the LeaderAndIsrRequest recreates the partition with 
getOrCreatePartition, and therefore future fetch requests hits it and throws 
that error code.

I think the correct thing to do is to not send LeaderAndIsrRequest if the topic 
is being deleted when bring replica to an offline state, as it is not necessary 
anymore.


> On April 28, 2014, 7:43 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/controller/KafkaController.scala, line 652
> > <https://reviews.apache.org/r/20745/diff/1/?file=569161#file569161line652>
> >
> > did you make this change since onControllerResignation already holds 
> > the same lock? 
> > 
> > In general, for tricky bugs like deadlock, it is a good idea to explain 
> > all the significant changes, so it is easier to review.

The latest fix for the deadlock decouples the await topic deletion with a new 
lock internal to delete topic manager, so that during the shutdown the await on 
the topic deletion condition and resume since it's not based on the controller 
lock anymore.

We're still seeing occasional deadlock hanging problem, and with investigation 
Jun and I believe it's because if the topic deletion resumes and right before 
it acquires the controller lock, if a shutdown happens and acquired the lock, 
the delete topic thread will be waiting forever as the shutdown also blocks and 
waits.

There are a variety of fixes, but I felt one of the cleaner fix is to shutdown 
the delete topic manager outside of the controller lock as it doesn't really 
require it anymore. Once delete topic thread shuts down then we can proceed 
holding the controller lock and finish the broker shutdown. 


> On April 28, 2014, 7:43 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, line 403
> > <https://reviews.apache.org/r/20745/diff/1/?file=569163#file569163line403>
> >
> > * delete topic wasn't initiated for the given topic
> > 
> > Nevertheless, it seems like the loop goes over topics that are queued 
> > up for deletion, so is this comment right?

There isn't really actual bug here, but I was trying to fix the comments and 
logic as if a topic was queued to be deleted and it's the first time calling 
delete topic thread, the current code and comment seems to suggest all topics 
that are queued that is not in completed has one failed replica. But it could 
be that it is just queued up to be deleted the first time.

The effect is that in the logs you will see that the current code will log that 
it attempts to retry topic deletion again, while it's just being deleted the 
first time. 

I was trying to say that this is the first time the topic has been queued for 
deletion and not resumed to be completed or retried. I'll try to rewrite it.


- Timothy


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


On April 27, 2014, 6:43 a.m., Timothy Chen wrote:
> 
> ---
> Th

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

(Updated April 28, 2014, 9:48 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Fix delete topic tests and deadlock


Diffs (updated)
-

  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0b668f230c8556fdf08654ce522a11847d0bf39b 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

(Updated April 29, 2014, 12:08 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Fix delete topic tests and deadlock


Diffs (updated)
-

  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0b668f230c8556fdf08654ce522a11847d0bf39b 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

> On April 29, 2014, 5:02 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, lines 
> > 209-212
> > <https://reviews.apache.org/r/20745/diff/3/?file=569935#file569935line209>
> >
> > Is it better to do the check here or in the caller?

Hmm, at first glance not sure why it makes sense to propagate 
LeaderAndIsrRequest when a partition is being deleted, but moving it just to 
the delete topic path is probably the least impact. I think I'll move it to the 
caller for now.


> On April 29, 2014, 5:02 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, lines 
> > 385-386
> > <https://reviews.apache.org/r/20745/diff/3/?file=569938#file569938line385>
> >
> > Do we need to check here at all since the caller of doWork() already 
> > does the check?

Good point, since isRunning must be set to false before it can shut down this 
is redudant.


> On April 29, 2014, 5:02 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, lines 
> > 412-413
> > <https://reviews.apache.org/r/20745/diff/3/?file=569938#file569938line412>
> >
> > Do we need to introduce isAnyReplicaInState? replicasInState() seems 
> > more general and we can just check the output to implement 
> > isAnyReplicaInState.

I added this because the logic here only want to see if there is any replica 
that is in a specific state, and thought returning the whole list is a waste as 
I see internally it's constructing a new set. From profiling other Scala code I 
see that any set constructing is quite expensive as it's using immutable sets. 
It is more clean to only expose one API, but I thought the trade off might be 
worth it. More thoughts?


> On April 29, 2014, 5:02 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala, lines 86-91
> > <https://reviews.apache.org/r/20745/diff/3/?file=569942#file569942line86>
> >
> > There seems to be no guarantee that the delete topic process is 
> > completed before the controller was shutdown. So, I am not sure how 
> > reliable the test is.

It's actually the reverse, where it tries to shutdown controller while the 
delete topic process is still in progress. Currently there is no way to 
gurantee that the delete topic can halt until some condition happens, so it 
might be not guranteed.


- Timothy


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


On April 29, 2014, 12:08 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> ---
> 
> (Updated April 29, 2014, 12:08 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1397
> https://issues.apache.org/jira/browse/KAFKA-1397
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Fix delete topic tests and deadlock
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 0e47dac8cbf65a86d053a3371a18af467afd70ae 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> e4bc2439ce1933c7c7571d255464ee678226a6cb 
>   core/src/main/scala/kafka/log/LogManager.scala 
> ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 0b668f230c8556fdf08654ce522a11847d0bf39b 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 9c29e144bba2c9bafa91941b6ca5c263490693b3 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
> b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 
> 
> Diff: https://reviews.apache.org/r/20745/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

(Updated April 30, 2014, 9:55 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
---

KAFKA-1397: Fix delete topic tests and deadlock


Diffs (updated)
-

  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/KafkaApis.scala 
0b668f230c8556fdf08654ce522a11847d0bf39b 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
11c20cee83fda9a492156674d351a0096b13fd99 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 
5c487968014b56490eb2bc876cef1c52efd1cdad 
  core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
384c74e7c3985abff864e61dea5e530dbd189d5d 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

> On May 1, 2014, 6:26 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, lines 
> > 394-396
> > <https://reviews.apache.org/r/20745/diff/4/?file=571881#file571881line394>
> >
> > It seems this test is not necessary since the outer loop will detect 
> > that too.

It's actually necessary since isRunning might been set to false while it woke 
up from the condition, in that case we don't want it to still try to acquire 
the controller lock but to bail early.


> On May 1, 2014, 6:26 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/ReplicaManager.scala, lines 128-136
> > <https://reviews.apache.org/r/20745/diff/4/?file=571885#file571885line128>
> >
> > Do we still need to do this now that we make sure that if a replica is 
> > deleted, the controller will never send a LeaderAndIsrRequest to this 
> > replica until the topic is deleted?

We need this as described in the comment that if a broker that was shutdown 
during the topic deletion and brought back up, the 
stopReplicaRequest(deletePartition=true) without this extra log manager cleanup 
will never delete the log and the folders associated with as ReplicaManager 
doesn't hold that information anymore as we don't send the leader and isr 
request again as part of the retry.


> On May 1, 2014, 6:26 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala, lines 86-94
> > <https://reviews.apache.org/r/20745/diff/4/?file=571886#file571886line86>
> >
> > This is not going to be reliable since we can't be sure whether the 
> > controller has completed the deletion before the failover.

Ok removing the test for now.


> On May 1, 2014, 6:26 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala, lines 239-240
> > <https://reviews.apache.org/r/20745/diff/4/?file=571886#file571886line239>
> >
> > This test seems to be the same as 
> > testPartitionReassignmentDuringDeleteTopic().

Just the order of requests, but as well cannot gurantee what is processed first 
as they're just asynchronous. Will remove.


- Timothy


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


On April 30, 2014, 9:55 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> ---
> 
> (Updated April 30, 2014, 9:55 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1397
> https://issues.apache.org/jira/browse/KAFKA-1397
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1397: Fix delete topic tests and deadlock
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
> 919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
> 0e47dac8cbf65a86d053a3371a18af467afd70ae 
>   core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
> e4bc2439ce1933c7c7571d255464ee678226a6cb 
>   core/src/main/scala/kafka/log/LogManager.scala 
> ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 0b668f230c8556fdf08654ce522a11847d0bf39b 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 11c20cee83fda9a492156674d351a0096b13fd99 
>   core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
> 9c29e144bba2c9bafa91941b6ca5c263490693b3 
>   core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 
> 5c487968014b56490eb2bc876cef1c52efd1cdad 
>   core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala 
> b1c4ce9f66aa86c68f2e6988f67546fa63ed1f9b 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> 384c74e7c3985abff864e61dea5e530dbd189d5d 
> 
> Diff: https://reviews.apache.org/r/20745/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

(Updated May 1, 2014, 10:54 p.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1397: Fix delete topic tests and deadlock


Diffs (updated)
-

  core/src/main/scala/kafka/admin/AdminUtils.scala 
36ddeb44490e8343a4e8056c45726ac660e4b2f9 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
11c20cee83fda9a492156674d351a0096b13fd99 
  core/src/main/scala/kafka/utils/ShutdownableThread.scala 
cf8adc9f468f4d6f01d1303efe39a3ec6f3d9b53 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 
5c487968014b56490eb2bc876cef1c52efd1cdad 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
49c7790c995bb2e79322eb148ab80d0dcccefed4 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20745: Patch for KAFKA-1397

[Timothy Chen]

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

(Updated May 2, 2014, 1:12 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1397: Fix delete topic tests and deadlock


Diffs (updated)
-

  core/src/main/scala/kafka/admin/AdminUtils.scala 
36ddeb44490e8343a4e8056c45726ac660e4b2f9 
  core/src/main/scala/kafka/controller/ControllerChannelManager.scala 
919aeb26f93d2fc34d873cfb3dbfab7235a9d635 
  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 
0e47dac8cbf65a86d053a3371a18af467afd70ae 
  core/src/main/scala/kafka/controller/TopicDeletionManager.scala 
e4bc2439ce1933c7c7571d255464ee678226a6cb 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/ReplicaManager.scala 
11c20cee83fda9a492156674d351a0096b13fd99 
  core/src/main/scala/kafka/utils/ShutdownableThread.scala 
cf8adc9f468f4d6f01d1303efe39a3ec6f3d9b53 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
9c29e144bba2c9bafa91941b6ca5c263490693b3 
  core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala 
5c487968014b56490eb2bc876cef1c52efd1cdad 
  core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala 
014e9644dba1d65142f6a9abc745858870a46230 
  core/src/test/scala/unit/kafka/utils/TestUtils.scala 
49c7790c995bb2e79322eb148ab80d0dcccefed4 

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


Testing
---


Thanks,

Timothy Chen

Re: Review Request 20718: Patch for KAFKA-1384

[Timothy Chen]

> On May 2, 2014, 1:09 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, lines 108-109
> > <https://reviews.apache.org/r/20718/diff/2/?file=569132#file569132line108>
> >
> > Will that override the state to broker even though it's set to 
> > controller?

The controller only sets the state when it is elected, and from the server 
startup steps it will set as broker state first, and once the controller starts 
up and the elect happens then it will be set to controller state.


> On May 2, 2014, 1:09 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServerStartable.scala, lines 55-58
> > <https://reviews.apache.org/r/20718/diff/2/?file=569133#file569133line55>
> >
> > What's the intention of this method?

This is to allow custom state that is not defined in the enums. One example is 
in our custom kafka server startable we can inject more states that is specific 
to different use cases.


- Timothy


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


On April 26, 2014, 7:09 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20718/
> ---
> 
> (Updated April 26, 2014, 7:09 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1384
> https://issues.apache.org/jira/browse/KAFKA-1384
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-1384: Logging kafka state metric
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 
> 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/log/Log.scala 
> f20c232e2daa63a10d91b965af52801af656477c 
>   core/src/main/scala/kafka/log/LogManager.scala 
> ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 
> acda52b801714bcc182edc0ced925f0e4b493fc1 
> 
> Diff: https://reviews.apache.org/r/20718/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 20718: Patch for KAFKA-1384

[Timothy Chen]

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

(Updated May 2, 2014, 1:47 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1384: Logging kafka state metric


Diffs (updated)
-

  core/src/main/scala/kafka/controller/KafkaController.scala 
933de9dd324c7086efe6aa610335ef370d9e9c12 
  core/src/main/scala/kafka/log/Log.scala 
b7bc5ffdecba7221b3d2e4bca2b0c703bc74fa12 
  core/src/main/scala/kafka/log/LogManager.scala 
ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
  core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaServer.scala 
c208f83bed7fb91f07fae42f2b66892e6d46fecc 
  core/src/main/scala/kafka/server/KafkaServerStartable.scala 
acda52b801714bcc182edc0ced925f0e4b493fc1 

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


Testing
---


Thanks,

Timothy Chen