[Broker] Check allowAutoSubscriptionCreation when creating init sub - #14458
Merged
Conversation
Motivation In apache#13355, we added support for creating initial subscription when creating the producer. But the broker didn't check if the subscription can be created automatically. The initial subscription will be created even if the `allowAutoSubscriptionCreation` is disabled. Modification * Check `allowAutoSubscriptionCreation` when creating the initial subscription. Signed-off-by: Zike Yang <zkyang@streamnative.io>
github-actions
Bot
added
the
doc-required
Jason918
reviewed
Feb 25, 2022
Member
|
@codelipenghui - this seems like a missing component for PIP 124, which made it into |
michaeljmarshall
requested review from
Jason918,
codelipenghui,
eolivelli and
michaeljmarshall
February 25, 2022 05:51
michaeljmarshall
requested changes
Feb 25, 2022
Contributor
|
I have added release/2.10.1 just to make sure I will not miss to cherry-pick this one, after cherry-picking I will change to 2.10.0, |
Signed-off-by: Zike Yang <zkyang@streamnative.io>
Signed-off-by: Zike Yang <zkyang@streamnative.io>
RobertIndie
added a commit
to RobertIndie/pulsar
that referenced
this pull request
Feb 25, 2022
…t sub Motivation This PR is the doc for apache#14458 Modification * Add doc for check `allowAutoSubscriptionCreation` when creating init sub Signed-off-by: Zike Yang <zkyang@streamnative.io>
1 task
Signed-off-by: Zike Yang <zkyang@streamnative.io>
Signed-off-by: Zike Yang <zkyang@streamnative.io>
github-actions
Bot
added
the
doc
codelipenghui
approved these changes
Feb 25, 2022
michaeljmarshall
approved these changes
Feb 25, 2022
| log.warn("[{}] {} initialSubscriptionName: {}, topic: {}", | ||
| remoteAddress, msg, initialSubscriptionName, topicName); | ||
| commandSender.sendErrorResponse(requestId, | ||
| ServerError.NotAllowedError, msg); |
Member
There was a problem hiding this comment.
Note that the consequence of sending this NotAllowedError is that the consumer will indefinitely send messages back to the broker to be redelivered. I think this is a sane default since it means messages won't be lost. I just want to make sure we agree on this design.
Signed-off-by: Zike Yang <zkyang@streamnative.io>
Jason918
approved these changes
Feb 26, 2022
yuruguo
approved these changes
Feb 26, 2022
codelipenghui
pushed a commit
that referenced
this pull request
Mar 1, 2022
…#14458) Master Issue: #13408 ### Motivation In #13355, we added support for creating initial subscription when creating the producer. But the broker didn't check if the subscription can be created automatically. The initial subscription will be created even if the `allowAutoSubscriptionCreation` is disabled. (cherry picked from commit db25438)
Anonymitaet
added
doc-complete
Nicklee007
pushed a commit
to Nicklee007/pulsar
that referenced
this pull request
Apr 20, 2022
…apache#14458) Master Issue: apache#13408 ### Motivation In apache#13355, we added support for creating initial subscription when creating the producer. But the broker didn't check if the subscription can be created automatically. The initial subscription will be created even if the `allowAutoSubscriptionCreation` is disabled.
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
8 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.
Master Issue: #13408
Motivation
In #13355, we added support for creating initial subscription when creating the producer. But the broker didn't check if the subscription can be created automatically. The initial subscription will be created even if the
allowAutoSubscriptionCreationis disabled.Modifications
allowAutoSubscriptionCreationwhen creating the initial subscription.Verifying this change
This change is already covered by existing tests, such as testInitialSubscriptionCreationWithAutoCreationDisable.
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
doc