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

[feature][broker] Support autoSubscriptionCreation on topic level #17720

Merged
merged 1 commit into from
Nov 2, 2022
Merged

[feature][broker] Support autoSubscriptionCreation on topic level #17720

merged 1 commit into from
Nov 2, 2022

Conversation

yuruguo
Copy link
Contributor

@yuruguo yuruguo commented Sep 19, 2022

Motivation

Current pulsar only support to set autoSubscriptionCreation on broker and namespace level, but we can further set this policy on the topic level.

Modifications

  • Add admin api for topic to set/get/remove autoSubscriptionCreation
  • handleSubscribe in ServerCnx need to check autoSubscriptionCreation policy of topic

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 19, 2022
@yuruguo yuruguo self-assigned this Sep 19, 2022
@yuruguo yuruguo changed the title [broker][admin] Support to set/get/remove autoSubscriptionCreation on topic [feature][broker] Support autoSubscriptionCreation on topic level Sep 21, 2022
@yuruguo yuruguo added this to the 2.12.0 milestone Sep 21, 2022
@yuruguo yuruguo added type/feature The PR added a new feature or issue requested a new feature area/broker labels Sep 21, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@d7c09be). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #17720   +/-   ##
=========================================
  Coverage          ?   23.64%           
  Complexity        ?     4757           
=========================================
  Files             ?      685           
  Lines             ?    67385           
  Branches          ?     7218           
=========================================
  Hits              ?    15930           
  Misses            ?    49096           
  Partials          ?     2359           
Flag Coverage Δ
unittests 23.64% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM
But BrokerService#isAllowAutoSubscriptionCreation(org.apache.pulsar.common.naming.TopicName) can be optimized with HierarchyTopicPolicies in future PR.

@yuruguo yuruguo merged commit 681d51d into apache:master Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs ready-to-test type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants