Skip to content
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

[FLINK-32020] Enable Dynamic Partition Discovery by Default in Kafka Source based on FLIP-288 #40

Closed
wants to merge 4 commits into from

Conversation

loserwang1024
Copy link
Contributor

What is the purpose of the change

As described in FLIP-288, dynamic partition discovery is disabled by default, and users have to specify the interval of discovery in order to turn it on. It will be better if enable partition discovery by default.

Brief change log

  • Kafka table source: "scan.topic-partition-discovery.interval" will be set to 5 minutes by default, aligned with the default value of "metadata.max.age.ms" in Kafka consumer.
  • Kafka stream source: "partition.discovery.interval.ms" will be set to 5 minutes by default, with discovery interval set to 5 minutes.

Verifying this change

  • See test unit.
  • Start flink with kafka source connector by default, then add kafka partition after a period of time to see if the new partition can be read.

… Default in Kafka Stream Source based on FLIP-288
… Default in Kafka Table/SQL Source based on FLIP-288
Copy link
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loserwang1024 Thanks for the PR! The logic looks good to me overall. Could you add another test case that validates disabling dynamic partition discovery?

Copy link
Contributor

@PatrickRen PatrickRen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loserwang1024 Thanks for the update. I left a tiny comment about the new test case you added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants