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/09 02:50:17 UTC
Review Request 20143: Patch for KAFKA-1363
-----------------------------------------------------------
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
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20143/#review40200
-----------------------------------------------------------
Ship it!
Ship It!
- Jun Rao
On April 9, 2014, 6:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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
Posted by Jun Rao <ju...@gmail.com>.
> On April 10, 2014, 6:24 a.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, line 104
> > <https://reviews.apache.org/r/20143/diff/3/?file=553966#file553966line104>
> >
> > Why do we need to call this function here?
This is needed to unblock awaitTopicDeletionNotification() in the DeleteTopicThread.
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20143/#review39989
-----------------------------------------------------------
On April 9, 2014, 6:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20143/#review39989
-----------------------------------------------------------
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20143/#comment72778>
Why do we need to call this function here?
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20143/#comment72779>
Is ShutdownableThread to be default as isInterruptible?
- Guozhang Wang
On April 9, 2014, 6:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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 20143: Patch for KAFKA-1363
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20143/#review39900
-----------------------------------------------------------
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20143/#comment72646>
Can we remove the debug statements? Seem only relevant to some one-off troubleshooting exercise.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20143/#comment72647>
Reading topicsToBeDeleted outside the controllerLock can cause a race condition. Since the thread is only notified once to start the topic deletion, it might miss acting on the notification. The effect would be that the user would notice that the delete topic command had no effect and would have to retry. This violates the guarantee that the current delete topic functionality gives which is - if delete topic is issued, it will always complete.
core/src/main/scala/kafka/controller/TopicDeletionManager.scala
<https://reviews.apache.org/r/20143/#comment72648>
Same for these debug statements.
- Neha Narkhede
On April 9, 2014, 12:52 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> 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
> -----
>
> 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
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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