> On March 18, 2014, 10:44 p.m., Joel Koshy wrote: > > The delete topic thread will never wake up if delete topic is disabled. > > Nevertheless, could we also check if delete topic is enabled before trying > > to start the deletetopicmanager (and the associated delete topic thread)?
hmm.. actually the thread will never start since the all the public APIs (including start) of TopicDeletionManager with the delete.topic.enable flag. So this should not be an issue. > On March 18, 2014, 10:44 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, line 176 > > <https://reviews.apache.org/r/19379/diff/1/?file=526469#file526469line176> > > > > Should be true Ya, so actually what we return from here when delete topic is disabled doesn't matter since this API is only relevant if topic has been queued up for deletion. Anyhow, I will change it if it is easier to understand. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19379/#review37654 ----------------------------------------------------------- On March 18, 2014, 10:32 p.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19379/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 10:32 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1311 > https://issues.apache.org/jira/browse/KAFKA-1311 > > > Repository: kafka > > > Description > ------- > > 1. Removed --delete from topic command 2. Added config to turn off delete > topic (delete.topic.enable) > > The reason I chose protecting all public APIs of TopicDeletionManager instead > of protecting everything that calls those APIs is only to contain the change. > It will be a more widespread and hence risky change to do the latter. Let me > know if people think otherwise. > > > 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 > >
