Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

Fix some corner cases not handled well for CreateTopics request#592

Merged
jiazhai merged 3 commits intostreamnative:masterfrom
BewareMyPower:bewaremypower/fix-create-topic
Jun 28, 2021
Merged

Fix some corner cases not handled well for CreateTopics request#592
jiazhai merged 3 commits intostreamnative:masterfrom
BewareMyPower:bewaremypower/fix-create-topic

Conversation

@BewareMyPower
Copy link
Copy Markdown
Collaborator

Fixes #591

Motivation

Kafka Connect use Admin#createTopics and check the TopicExistsException exception for existed topics. See https://github.com/apache/kafka/blob/ef3cd68c852daf21d8e207fa8e21c8770f4041d7/connect/runtime/src/main/java/org/apache/kafka/connect/util/TopicAdmin.java#L394 for details.

However, currently Admin#createTopics will return an UnknownServerException for any failure when create topics. In addition, the handler for CreateTopics request has other problems.

  1. The default partition number (-1) is not handled, which may throw exception from createPartitionedTopicAsync.
  2. If topic name is invalid, the future won't be completed until timeout. See Fix flaky testCreateInvalidTopics #361, TimeoutException will be thrown in this case.

Modifications

  1. When the topic to create already exists, return a TopicExistsException.
  2. Add a tryComplete method to complete a future of topic creation that may also complete the future of response.
  3. Add related tests.

@BewareMyPower
Copy link
Copy Markdown
Collaborator Author

FYI @Krishsocgen you can try this patch for your Kafka Connect.

@BewareMyPower BewareMyPower changed the title Fix some corner cases not handled well for CreateTopics request [WIP] Fix some corner cases not handled well for CreateTopics request Jun 25, 2021
@BewareMyPower
Copy link
Copy Markdown
Collaborator Author

The Kafka Streams tests may fail after this PR, I'll take a look.

@BewareMyPower BewareMyPower force-pushed the bewaremypower/fix-create-topic branch from 4a4ab98 to 05c962c Compare June 25, 2021 04:18
@BewareMyPower BewareMyPower changed the title [WIP] Fix some corner cases not handled well for CreateTopics request Fix some corner cases not handled well for CreateTopics request Jun 25, 2021
@jiazhai jiazhai merged commit d69c256 into streamnative:master Jun 28, 2021
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-create-topic branch June 28, 2021 03:50
BewareMyPower added a commit that referenced this pull request Jun 29, 2021
Fixes #591 

### Motivation

Kafka Connect use `Admin#createTopics` and check the `TopicExistsException` exception for existed topics. See https://github.com/apache/kafka/blob/ef3cd68c852daf21d8e207fa8e21c8770f4041d7/connect/runtime/src/main/java/org/apache/kafka/connect/util/TopicAdmin.java#L394 for details.

However, currently `Admin#createTopics` will return an `UnknownServerException` for any failure when create topics. In addition, the handler for CreateTopics request has other problems.

1. The default partition number (-1) is not handled, which may throw exception from `createPartitionedTopicAsync`.
2. If topic name is invalid, the future won't be completed until timeout. See #361, TimeoutException will be thrown in this case.

### Modifications

1. When the topic to create already exists, return a `TopicExistsException`.
2. Add a `tryComplete` method to complete a future of topic creation that may also complete the future of response.
3. Add related tests.

=====
* Fix some bugs for create topic request
* Fix KStreamAggregationTest
* Catch KoPTopicException instead of RuntimeException
BewareMyPower added a commit that referenced this pull request Jun 29, 2021
Fixes #591

Kafka Connect use `Admin#createTopics` and check the `TopicExistsException` exception for existed topics. See https://github.com/apache/kafka/blob/ef3cd68c852daf21d8e207fa8e21c8770f4041d7/connect/runtime/src/main/java/org/apache/kafka/connect/util/TopicAdmin.java#L394 for details.

However, currently `Admin#createTopics` will return an `UnknownServerException` for any failure when create topics. In addition, the handler for CreateTopics request has other problems.

1. The default partition number (-1) is not handled, which may throw exception from `createPartitionedTopicAsync`.
2. If topic name is invalid, the future won't be completed until timeout. See #361, TimeoutException will be thrown in this case.

1. When the topic to create already exists, return a `TopicExistsException`.
2. Add a `tryComplete` method to complete a future of topic creation that may also complete the future of response.
3. Add related tests.

=====
* Fix some bugs for create topic request
* Fix KStreamAggregationTest
* Catch KoPTopicException instead of RuntimeException
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Kafka Connect failed to restart on KoP

3 participants