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

Fix create consumer on partitioned topic while disable topic auto creation. #5572

Merged
merged 6 commits into from Jan 9, 2020

Conversation

@codelipenghui
Copy link
Contributor

codelipenghui commented Nov 6, 2019

Fixes #5565

Motivation

Currently, disable the topic auto creation will cause consumer create failed on a partitioned topic. Since the partitioned topic is already created, so we should handle the topic partition create when disable the topic auto creation.

Modifications

By default, create partitioned topics also try to create all partitions, and if create partitions failed, users can use create-missed-partitions to repair.

If users already have a partitioned topic without created partitions, can also use create-missed-partitions to repair.

Verifying this change

Add new unit tests for this change

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 6, 2019

@wolfstudy if this PR can complete before cut 2.4.2, please considering include it, thanks.

@wolfstudy wolfstudy added this to the 2.4.2 milestone Nov 6, 2019
Copy link
Member

sijie left a comment

I don't think that is the right fix for this problem. The purpose of disabling topic auto-creation is to prevent consumers and producers creating topics.

I think the correct fix should be improving the pulsar-admin tool to create and delete partitions when creating or updating a partitioned topic.

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 12, 2019

run integration tests

@codelipenghui codelipenghui requested a review from sijie Nov 12, 2019
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 12, 2019

run java8 tests

@wolfstudy

This comment has been minimized.

Copy link
Member

wolfstudy commented Nov 13, 2019

@codelipenghui l will change the Milestone to 2.4.3. So we can cut 2.4.2 and if needed
2.4.3 in a few weeks.

@wolfstudy wolfstudy removed this from the 2.4.2 milestone Nov 13, 2019
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 13, 2019

@wolfstudy Thanks

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 13, 2019

run java8 tests

1 similar comment
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 16, 2019

run java8 tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 19, 2019

run java8 tests

3 similar comments
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 19, 2019

run java8 tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 19, 2019

run java8 tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 19, 2019

run java8 tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 19, 2019

@sijie Please help review this PR again

@codelipenghui codelipenghui added this to the 2.5.0 milestone Nov 25, 2019
@sijie

This comment has been minimized.

Copy link
Member

sijie commented Nov 25, 2019

@codelipenghui are you going to include this fix in 2.5.0 or shall we move it to 2.5.1 or 2.6.0?

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Nov 27, 2019

@sijie sijie removed this from the 2.5.0 milestone Nov 27, 2019
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Dec 24, 2019

@sijie I have addressed your comments, PTAL.

@sijie
sijie approved these changes Dec 24, 2019
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Dec 24, 2019

run java8 tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Dec 24, 2019

run java8 tests
run integration tests

@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Dec 24, 2019

run integration tests

1 similar comment
@codelipenghui

This comment has been minimized.

Copy link
Contributor Author

codelipenghui commented Dec 24, 2019

run integration tests

@codelipenghui codelipenghui merged commit 602f1c2 into apache:master Jan 9, 2020
10 of 18 checks passed
10 of 18 checks passed
cpp-tests cpp-tests
Details
backwards-compatibility backwards-compatibility
Details
process process
Details
sql sql
Details
thread thread
Details
License check License check
Details
unit-test-flaky unit-test-flaky
Details
unit-tests unit-tests
Details
cli
Details
function-state
Details
messaging
Details
schema
Details
standalone
Details
tiered-filesystem
Details
tiered-jcloud
Details
Jenkins: C++ / Python Tests SUCCESS
Details
Jenkins: Integration Tests SUCCESS
Details
Jenkins: Java 8 - Unit Tests SUCCESS
Details
@sijie sijie added the release/2.5.1 label Jan 22, 2020
@sijie sijie added this to the 2.6.0 milestone Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.