-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-9945: TopicCommand should support --if-exists and --if-not-exists when --bootstrap-server is used #8598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,7 +245,17 @@ object TopicCommand extends Logging { | |
|
|
||
| newTopic.configs(configsMap) | ||
| val createResult = adminClient.createTopics(Collections.singleton(newTopic)) | ||
| createResult.all().get() | ||
| try { | ||
| createResult.all().get() | ||
| } catch { | ||
| case e: ExecutionException => { | ||
| val cause = e.getCause | ||
| if (cause.isInstanceOf[TopicExistsException] || topic.ifTopicDoesntExist()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we re-throw all exceptions except |
||
| throw e | ||
| } | ||
| } | ||
| } | ||
|
|
||
| println(s"Created topic ${topic.name}.") | ||
| } else { | ||
| throw new IllegalArgumentException(s"Topic ${topic.name} already exists") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently, we already verify the existence of the topic prior to creating it. Should we also handle |
||
|
|
@@ -257,21 +267,50 @@ object TopicCommand extends Logging { | |
| } | ||
|
|
||
| override def alterTopic(opts: TopicCommandOptions): Unit = { | ||
| val topic = new CommandTopicPartition(opts) | ||
| val topics = getTopics(opts.topic, opts.excludeInternalTopics) | ||
| ensureTopicExists(topics, opts.topic) | ||
| val topicsInfo = adminClient.describeTopics(topics.asJavaCollection).values() | ||
| adminClient.createPartitions(topics.map {topicName => | ||
| if (topic.hasReplicaAssignment) { | ||
| val startPartitionId = topicsInfo.get(topicName).get().partitions().size() | ||
| val newAssignment = { | ||
| val replicaMap = topic.replicaAssignment.get.drop(startPartitionId) | ||
| new util.ArrayList(replicaMap.map(p => p._2.asJava).asJavaCollection).asInstanceOf[util.List[util.List[Integer]]] | ||
| if(opts.topicConfig.isDefined || opts.configsToDelete.isDefined) { | ||
| throw new RuntimeException("Using --config or --delete-config is not supported " + | ||
| "when altering a topic via the broker API. Use kafka-configs.sh instead.") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Extra space after the first |
||
| } | ||
| val tp = new CommandTopicPartition(opts) | ||
| if (tp.hasPartitions) { | ||
| println("WARNING: If partitions are increased for a topic that has a key, the partition " + | ||
| "logic or ordering of the messages will be affected") | ||
| val topicDescription = try { | ||
| adminClient.describeTopics(Collections.singleton(tp.name)). | ||
| all().get().get(tp.name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: unnecessary spaces before |
||
| } catch { | ||
| case e: ExecutionException => { | ||
| val cause = e.getCause | ||
| if (cause.isInstanceOf[TopicExistsException] && opts.ifExists) { | ||
| println(s"Ignoring non-existent topic ${tp.name}.") | ||
| return | ||
| } else { | ||
| throw e | ||
| } | ||
| } | ||
| topicName -> NewPartitions.increaseTo(topic.partitions.get, newAssignment) | ||
| } | ||
| if (topicDescription.partitions().size() == tp.partitions.get) { | ||
| println(s"Topic ${tp.name} already has ${tp.partitions.get} partitions. " + | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: extra space after the |
||
| "Nothing to do.") | ||
| return | ||
| } else if (topicDescription.partitions().size() > tp.partitions.get) { | ||
| println(s"Topic ${tp.name} already has ${topicDescription.partitions().size()} " + | ||
| s"partitions, which is more than the specified ${tp.partitions.get}. " + | ||
| "The number of partitions can't be reduced by this tool.") | ||
| return | ||
| } | ||
| val newAssignment = if (tp.hasReplicaAssignment) { | ||
| val startPartitionId = topicDescription.partitions().size() | ||
| val newAssignment = startPartitionId.until(tp.partitions.get).map { | ||
| i => tp.replicaAssignment.get.get(i).get.map(_.asInstanceOf[Integer]).asJava | ||
| } | ||
| NewPartitions.increaseTo(tp.partitions.get, newAssignment.asJava) | ||
| } else { | ||
| topicName -> NewPartitions.increaseTo(topic.partitions.get) | ||
| }}.toMap.asJava).all().get() | ||
| NewPartitions.increaseTo(tp.partitions.get) | ||
| } | ||
| adminClient.createPartitions(Collections.singletonMap(tp.name, newAssignment)). | ||
| all().get() | ||
| } | ||
| } | ||
|
|
||
| private def listAllReassignments(): Map[TopicPartition, PartitionReassignment] = { | ||
|
|
@@ -323,9 +362,19 @@ object TopicCommand extends Logging { | |
| } | ||
|
|
||
| override def deleteTopic(opts: TopicCommandOptions): Unit = { | ||
| val topics = getTopics(opts.topic, opts.excludeInternalTopics) | ||
| ensureTopicExists(topics, opts.topic) | ||
| adminClient.deleteTopics(topics.asJavaCollection).all().get() | ||
| try { | ||
| adminClient.deleteTopics(Collections.singletonList(opts.topic.get)).all().get() | ||
| } catch { | ||
| case e: ExecutionException => { | ||
| val cause = e.getCause | ||
| if (cause.isInstanceOf[TopicExistsException] && opts.ifExists) { | ||
| println(s"Ignoring non-existent topic ${opts.topic.get}.") | ||
| return | ||
| } else { | ||
| throw e | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override def getTopics(topicWhitelist: Option[String], excludeInternalTopics: Boolean = false): Seq[String] = { | ||
|
|
@@ -501,7 +550,7 @@ object TopicCommand extends Logging { | |
| * If set to true, the command will throw an exception if the topic with the | ||
| * requested name does not exist. | ||
| */ | ||
| private def ensureTopicExists(foundTopics: Seq[String], requestedTopic: Option[String], requireTopicExists: Boolean = true): Unit = { | ||
| private def ensureTopicExists(foundTopics: Seq[String], requestedTopic: Option[String], requireTopicExists: Boolean): Unit = { | ||
| // If no topic name was mentioned, do not need to throw exception. | ||
| if (requestedTopic.isDefined && requireTopicExists && foundTopics.isEmpty) { | ||
| // If given topic doesn't exist then throw exception | ||
|
|
@@ -639,9 +688,9 @@ object TopicCommand extends Logging { | |
| private val topicsWithOverridesOpt = parser.accepts("topics-with-overrides", | ||
| "if set when describing topics, only show topics that have overridden configs") | ||
| private val ifExistsOpt = parser.accepts("if-exists", | ||
| "if set when altering or deleting or describing topics, the action will only execute if the topic exists. Not supported with the --bootstrap-server option.") | ||
| "if set when altering or deleting or describing topics, the action will only execute if the topic exists.") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to update |
||
| private val ifNotExistsOpt = parser.accepts("if-not-exists", | ||
| "if set when creating topics, the action will only execute if the topic does not already exist. Not supported with the --bootstrap-server option.") | ||
| "if set when creating topics, the action will only execute if the topic does not already exist.") | ||
|
|
||
| private val disableRackAware = parser.accepts("disable-rack-aware", "Disable rack aware replica assignment") | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the parenthesis are not necessary. there is other cases when they can be removed.