You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Timothy Chen <tn...@apache.org> on 2014/04/27 08:43:26 UTC
Review Request 20745: Patch for KAFKA-1397
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
> 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:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated April 27, 2014, 6:43 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
Posted by Jun Rao <ju...@gmail.com>.
> 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?
>
> Timothy Chen wrote:
> 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.
>
Could you explain the situation in a bit more details? It would be good to include the sequence of LeaderIsr and StopReplica requests that the broker sees. In particular, I am not sure why a partition still exists in replica manager if a stop replica request with delete partition has already been received.
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41626
-----------------------------------------------------------
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
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41626
-----------------------------------------------------------
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20745/#comment75161>
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?
core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/20745/#comment75162>
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.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75158>
* 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?
core/src/main/scala/kafka/log/LogManager.scala
<https://reviews.apache.org/r/20745/#comment75159>
isLogExists -> doesLogExist
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75160>
Given that waitUntilTrue has been changed to fail the test, we can remove all the wrapper asserts
- Neha Narkhede
On April 27, 2014, 6:43 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated April 27, 2014, 6:43 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
Posted by Timothy Chen <tn...@apache.org>.
> 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
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41904
-----------------------------------------------------------
1. Should we restore the commented out test in ServerShutdownTest.scala?
core/src/main/scala/kafka/controller/ReplicaStateMachine.scala
<https://reviews.apache.org/r/20745/#comment75556>
Could you explain why we need to send metadata if the topic is being deleted?
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75583>
Do we need to change any comment? Also, I think those comments could probably be made clearer if we (1) separate out the normal cases with other cases such as broker failures and other conflicting concurrent operations; (2) describe the kind of requests sent to the brokers in each step.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75577>
It seems it's useful to extend the ShutdownableThread a bit. Currently, it has shutdown() which initiates the shutdown (by setting the flag and doing the interrupt if necessary) and waits until the thread completes the shutdown. It would be useful to add initiateShutdown() and waitShutdownComplete() (We can get rid of awaitShutdown()). Then, here, the shutdown sequence would be:
1. deleteTopicThread.inititateShutdown()
2. resumeTopicDeletionThread()
3. deleteTopicThread.waitShutdownCompelte().
This way, we make sure that deleteTopicThread can do at most 1 more loop after the first step is completed.
Also, perhaps it's clearer if we just add comment on each of the steps, instead of commenting everything at the beginning.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75581>
Perhaps it's better to call this TopicPartitionToBeDeleted? This method is not doing the same type of check as isTopicDeletionInProgress().
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75557>
It seems this test is not necessary since the outer loop will detect that too.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/20745/#comment75584>
This doesn't seem to be needed any more.
core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/20745/#comment75585>
This doesn't seem to be needed any more.
core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/20745/#comment75586>
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?
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75587>
We can just use the default larger timeout. This applies to other waitUntilTrue() usage. The only exception is that if we want to test sth that won't happen, using a smaller timeout will save time.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75588>
1000ms mentioned in the message is not consistent with the actual timeout.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75589>
This is not going to be reliable since we can't be sure whether the controller has completed the deletion before the failover.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75590>
We can just use the default timeout in waitUntilTrue.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75591>
This test seems to be the same as testPartitionReassignmentDuringDeleteTopic().
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75592>
If we want the add partition operation to block, should we shutdown the follower before calling addPartitions()?
- Jun Rao
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
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review42015
-----------------------------------------------------------
Not all comments from the previous round of review have been addressed. Could you make another pass? If they don't need to be addressed, could you add a comment?
- Jun Rao
On May 2, 2014, 1:12 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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 20745: Patch for KAFKA-1397
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review42195
-----------------------------------------------------------
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75940>
The follower could be the controller. So, we probably should shutdown a replica that's not the controller.
- Jun Rao
On May 5, 2014, 6:17 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 6:17 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/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 00bfba41fe008fe72c17fb788e3cada3daf8eb30
>
> Diff: https://reviews.apache.org/r/20745/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review42199
-----------------------------------------------------------
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75941>
Makes sense!
- Timothy Chen
On May 5, 2014, 9 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated May 5, 2014, 9 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/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 00bfba41fe008fe72c17fb788e3cada3daf8eb30
>
> Diff: https://reviews.apache.org/r/20745/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/
-----------------------------------------------------------
(Updated May 5, 2014, 9 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/server/ServerShutdownTest.scala 014e9644dba1d65142f6a9abc745858870a46230
core/src/test/scala/unit/kafka/utils/TestUtils.scala 00bfba41fe008fe72c17fb788e3cada3daf8eb30
Diff: https://reviews.apache.org/r/20745/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/
-----------------------------------------------------------
(Updated May 5, 2014, 6:17 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/server/ServerShutdownTest.scala 014e9644dba1d65142f6a9abc745858870a46230
core/src/test/scala/unit/kafka/utils/TestUtils.scala 00bfba41fe008fe72c17fb788e3cada3daf8eb30
Diff: https://reviews.apache.org/r/20745/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
> On May 5, 2014, 5:12 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 28-33
> > <https://reviews.apache.org/r/20745/diff/6/?file=573313#file573313line28>
> >
> > I thought you plan to remove isShuttingDown?
Odd I don't have this anymore in my box, but end up in my patch....
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review42058
-----------------------------------------------------------
On May 2, 2014, 8:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated May 2, 2014, 8:38 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/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 00bfba41fe008fe72c17fb788e3cada3daf8eb30
>
> Diff: https://reviews.apache.org/r/20745/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review42058
-----------------------------------------------------------
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75875>
Perhaps it's better to move them to after deleteTopicsThread.awaitShutdown() since only then we are certain that the delete topics thread is gone.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75876>
Perhaps this can be renamed as isPartitionToBeDeleted.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75877>
This should be debug level.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75882>
This comment is no longer valid.
core/src/main/scala/kafka/utils/ShutdownableThread.scala
<https://reviews.apache.org/r/20745/#comment75878>
I thought you plan to remove isShuttingDown?
core/src/main/scala/kafka/log/LogManager.scala
<https://reviews.apache.org/r/20745/#comment75798>
You can revert the changes in LogManager now.
- Jun Rao
On May 2, 2014, 8:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20745/
> -----------------------------------------------------------
>
> (Updated May 2, 2014, 8:38 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/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 00bfba41fe008fe72c17fb788e3cada3daf8eb30
>
> Diff: https://reviews.apache.org/r/20745/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/
-----------------------------------------------------------
(Updated May 2, 2014, 8:38 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/server/ServerShutdownTest.scala 014e9644dba1d65142f6a9abc745858870a46230
core/src/test/scala/unit/kafka/utils/TestUtils.scala 00bfba41fe008fe72c17fb788e3cada3daf8eb30
Diff: https://reviews.apache.org/r/20745/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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 20745: Patch for KAFKA-1397
Posted by Timothy Chen <tn...@apache.org>.
> On May 2, 2014, 12:26 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 35-36
> > <https://reviews.apache.org/r/20745/diff/5/?file=573213#file573213line35>
> >
> > We probably should check the return value of initiateShutdown().
I was thinking that even with concurrent calls since all calls end up just waiting on the shutdownLatch it should be safe to not check it.
Therefore shutdown call regardless of what state will end up waiting until the thread shutdown is completed.
> On May 2, 2014, 12:26 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 40-41
> > <https://reviews.apache.org/r/20745/diff/5/?file=573213#file573213line40>
> >
> > Instead of introducing isShuttingDown, could we just to compareAndSet on isRunning?
Good point, consolidated it.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41979
-----------------------------------------------------------
On May 2, 2014, 1:12 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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 20745: Patch for KAFKA-1397
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41979
-----------------------------------------------------------
core/src/main/scala/kafka/admin/AdminUtils.scala
<https://reviews.apache.org/r/20745/#comment75675>
Could we add a comment that checkBrokerAvailable is only used for testing purpose?
core/src/main/scala/kafka/controller/ReplicaStateMachine.scala
<https://reviews.apache.org/r/20745/#comment75680>
Could you add some comments on why we need to send updateMetadata request in the case?
core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/20745/#comment75697>
Could we just use logManager.getLog() instead of introducing doesLogExists()?
core/src/main/scala/kafka/utils/ShutdownableThread.scala
<https://reviews.apache.org/r/20745/#comment75685>
We probably should check the return value of initiateShutdown().
core/src/main/scala/kafka/utils/ShutdownableThread.scala
<https://reviews.apache.org/r/20745/#comment75684>
Instead of introducing isShuttingDown, could we just to compareAndSet on isRunning?
core/src/main/scala/kafka/utils/ShutdownableThread.scala
<https://reviews.apache.org/r/20745/#comment75687>
lower case "Shutting".
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75701>
It may be possible to design a reliable version of the test. We can (1) create a partition with 2 replicas, (2) shutdown one of the replicas that's not the controller, (3) initiate topic deletion (which will block since 1 replica is down), (4) failed the controller, (5) bring both brokers up, (6) verify topic is deleted.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75702>
The message should be "partition reassignment shouldn't complete".
- Jun Rao
On May 1, 2014, 10:54 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
> 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
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20745/#review41714
-----------------------------------------------------------
Some comments:
1. Should we restore the commented out test in ServerShutdownTest.scala?
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20745/#comment75277>
Is it better to do the check here or in the caller?
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75278>
The shutdown sequence is a bit complicated now. Could you add some comments to describe the sequence of actions?
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75282>
Do we need to check here at all since the caller of doWork() already does the check?
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20745/#comment75283>
Do we need to introduce isAnyReplicaInState? replicasInState() seems more general and we can just check the output to implement isAnyReplicaInState.
core/src/main/scala/kafka/server/KafkaApis.scala
<https://reviews.apache.org/r/20745/#comment75284>
It's better to do this in the ReplicaManager when handling the StopReplicaRequest. Then we don't have to expose LogManager to KafkaApis.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75285>
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.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75286>
The comment is wrong. We are shutting down the leader, not the controller.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75287>
Could we explicitly specify whether leaderIdOpt is for the old leader or the new leader in waitUntilLeaderIsElectedOrChanged()?
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75289>
I am not sure how reliable the test is since since by the time delete topic is started, the preferred leader election could have completed.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75291>
Again, this test is not reliable. There are multiple possible outcomes.
1. Delete topic completes before partition reassignment is started. In this case partition reassignment should fail. Then, we shouldn't verify the reassigned replicas since the topic doesn't exist.
2. Reassignment is started/completed before the topic deletion completes.
I am not sure which case this test is for.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75292>
Same comment as the above. I am not sure how we guarantee whether partition reassignment or topic deletion completes first.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/20745/#comment75296>
Not sure if this adds any value than the previous send.
- Jun Rao
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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