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.

Sign up for GitHub

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

Jump to bottom

[fix][broker] Fix inconsensus namespace policies by getPoliciesIfCached #20855

Merged
merged 11 commits into from
Jul 25, 2023
Merged

[fix][broker] Fix inconsensus namespace policies by getPoliciesIfCached #20855

merged 11 commits into from
Jul 25, 2023

Conversation

mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Jul 22, 2023 •
edited
Loading

Motivation

#12339 Change some metadata blocking calls to get from the cache. And then #14154 add the cache expires. It will introduce inconsistent namespace policies problem when the cache is missed.

Modifications

  • Deprecated All of getPoliciesIfCached related logic, then use xxxAsync instead of it.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

@mattisonchao mattisonchao self-assigned this Jul 22, 2023
@mattisonchao mattisonchao marked this pull request as ready for review July 22, 2023 18:20
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 22, 2023
@mattisonchao mattisonchao added this to the 3.2.0 milestone Jul 22, 2023
poorbarcode
poorbarcode reviewed Jul 24, 2023
View reviewed changes
Copy link
Contributor

@poorbarcode poorbarcode left a comment

Choose a reason for hiding this comment

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

Good catch, could you add a test?

@mattisonchao mattisonchao reopened this Jul 25, 2023
@mattisonchao mattisonchao requested a review from poorbarcode July 25, 2023 02:39
@mattisonchao
Copy link
Member Author

Good catch, could you add a test?

#20843 has already added a test to prove the cache problem. we should avoid using this API to get inconsistent namespace policies.

@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.1.0 Jul 25, 2023
@mattisonchao mattisonchao merged commit 563f929 into apache:master Jul 25, 2023
@mattisonchao mattisonchao mentioned this pull request Jul 25, 2023
Merged
@mattisonchao mattisonchao mentioned this pull request Jul 31, 2023
Merged
Technoboy- pushed a commit that referenced this pull request Aug 17, 2023
@mattisonchao @Technoboy-
[fix][broker] Fix inconsensus namespace policies by `getPoliciesIfCac…
15bacf8 
…hed` (#20855)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Technoboy- Technoboy- Technoboy- approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@poorbarcode poorbarcode poorbarcode approved these changes

@coderzc coderzc coderzc approved these changes

Assignees

@mattisonchao mattisonchao

Labels
area/broker cherry-picked/branch-2.10 cherry-picked/branch-2.11 cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.10.6 release/2.11.3 release/3.0.2
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mattisonchao @Technoboy- @codelipenghui @poorbarcode @coderzc @shibd