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

[improve][broker] cleanup the empty subscriptionAuthenticationMap in zk when revoke subscription permission #21696

Conversation

TakaHiR07
Copy link
Contributor

Motivation

This is a similar improve as #16815 . After we revoke subscription permission, we would see zk still remain many empty record.

企业微信截图_f30a9f1f-b0ed-441c-9eac-bf397c917009

Modifications

If isEmpty, remove the record in zk.

Verifying this change

  • Make sure that the change passes the CI checks.

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

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 Dec 8, 2023
@Technoboy- Technoboy- added this to the 3.2.0 milestone Dec 8, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #21696 (689e738) into master (6e18874) will increase coverage by 36.70%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #21696       +/-   ##
=============================================
+ Coverage     36.67%   73.37%   +36.70%     
- Complexity    12248    32756    +20508     
=============================================
  Files          1716     1893      +177     
  Lines        131166   140769     +9603     
  Branches      14330    15507     +1177     
=============================================
+ Hits          48104   103292    +55188     
+ Misses        76644    29365    -47279     
- Partials       6418     8112     +1694     
Flag Coverage Δ
inttests 24.17% <0.00%> (+0.02%) ⬆️
systests 24.67% <0.00%> (-0.06%) ⬇️
unittests 72.66% <100.00%> (+40.87%) ⬆️

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

Files Coverage Δ
...ker/authorization/PulsarAuthorizationProvider.java 68.61% <100.00%> (+42.91%) ⬆️

... and 1463 files with indirect coverage changes

Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

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

LGTM

@nodece nodece merged commit f427c90 into apache:master Dec 13, 2023
47 of 48 checks passed
Technoboy- added a commit that referenced this pull request Dec 14, 2023
…zk when revoke subscription permission (#21696)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwe Guo <technoboy@apache.org>
@horizonzy
Copy link
Member

The method getTlsFileForClient does not exist in branch-3.0, could you help to fix it? @TakaHiR07

@TakaHiR07
Copy link
Contributor Author

@horizonzy Fix getTlsFileForClient not exist in branch-3.0. #21744

Technoboy- added a commit that referenced this pull request Jan 3, 2024
…zk when revoke subscription permission (#21696)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwe Guo <technoboy@apache.org>
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 4, 2024
…zk when revoke subscription permission (apache#21696)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit b7d3a9c)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 8, 2024
…zk when revoke subscription permission (apache#21696)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit b7d3a9c)
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.

6 participants