[Transaction] Fix transaction system topic create in loop.#12749
Merged
congbobo184 merged 6 commits intoNov 13, 2021
Merged
Conversation
congbobo184
added
cherry-picked/branch-2.8
eolivelli
reviewed
Nov 11, 2021
|
|
||
| public static boolean isNotAllowedToCreateTopic(TopicName topicName) { | ||
| String topic = topicName.toString(); | ||
| return topic.startsWith(TopicName.get(TopicDomain.persistent.value(), |
Contributor
There was a problem hiding this comment.
what about creating a constant for this ? "TopicName.get(TopicDomain.persistent.value(), NamespaceName.SYSTEM_NAMESPACE, TRANSACTION_LOG_PREFIX).toString()"
| } | ||
| } | ||
|
|
||
| protected void validateTopicAllowdToCreate(TopicName topicName) { |
Contributor
There was a problem hiding this comment.
typo: validateCreateTopic
Contributor
|
Can you add my reproducer as a test case ? I am not sure that we will really fix the problem. Because we are not forbidding to create a topic with a system name. |
codelipenghui
added
release/2.8.3
release/2.9.1
and removed
cherry-picked/branch-2.8
eolivelli
requested changes
Nov 12, 2021
eolivelli
left a comment
eolivelli
left a comment
Contributor
There was a problem hiding this comment.
I left one last minor comment.
| || topic.endsWith(MLPendingAckStore.PENDING_ACK_STORE_SUFFIX); | ||
| } | ||
|
|
||
| public static boolean isNotAllowedToCreateTopic(TopicName topicName) { |
Contributor
There was a problem hiding this comment.
what about naming this method isTransactionInternalName ?
the meaning is that this is a reserved name
I see that you are using this method while listing the topic names, so "Create" is not good in that place
codelipenghui
approved these changes
Nov 12, 2021
added 2 commits
November 12, 2021 23:12
…ate_transaction_system_topic
Member
|
Confirmed w/ @codelipenghui, this PR is a bug fix, no need to update docs. |
Anonymitaet
removed
the
doc-required
eolivelli
pushed a commit
to eolivelli/pulsar
that referenced
this pull request
Nov 29, 2021
) fix apache#12727 ### Motivation Now transaction system topic can be created. ### Modifications we should not allow broker or user create by transaction system format topic. 1. checkout topic auto create. 2. admin create topic. ### Verifying this change add some test for it
eolivelli
pushed a commit
that referenced
this pull request
Dec 15, 2021
fix #12727 ### Motivation Now transaction system topic can be created. ### Modifications we should not allow broker or user create by transaction system format topic. 1. checkout topic auto create. 2. admin create topic. ### Verifying this change add some test for it (cherry picked from commit 2c4d913)
eolivelli
added
cherry-picked/branch-2.9
2 tasks
Merged
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #12727
Motivation
Now transaction system topic can be created.
Modifications
we should not allow broker or user create by transaction system format topic.
Verifying this change
add some test for it