[fix][broker] Fix namespace deletion if __change_events topic has not been created yet by nicoloboschi · Pull Request #18804 · apache/pulsar

nicoloboschi · 2022-12-07T14:58:37Z

Motivation

Sending the event to the __change_events topic is useless if the namespace is deleted (or in the process of).
Also during the namespace deletion procedure, it's not possible to create new topics so if it doesn't exist yet it throws an error.

Modifications

Skip sending events for topic policies if the namespace has been deleted

Verifying this change

Make sure that the change passes the CI checks.

Documentation

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

…created yet

codecov-commenter · 2022-12-08T08:57:54Z

Codecov Report

❌ Patch coverage is 62.50000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.23%. Comparing base (68ca60c) to head (ac72e17).
⚠️ Report is 3159 commits behind head on master.

Files with missing lines	Patch %	Lines
.../service/SystemTopicBasedTopicPoliciesService.java	60.86%	10 Missing and 8 partials ⚠️

❗ There is a different number of reports uploaded between BASE (68ca60c) and HEAD (ac72e17). Click for more details.

HEAD has 4 uploads less than BASE

Flag BASE (68ca60c) HEAD (ac72e17)

unittests 8 4

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #18804       +/-   ##
=============================================
- Coverage     50.05%   34.23%   -15.83%     
+ Complexity    11024     6518     -4506     
=============================================
  Files           703      623       -80     
  Lines         68814    59090     -9724     
  Branches       7378     6145     -1233     
=============================================
- Hits          34446    20227    -14219     
- Misses        30621    36194     +5573     
+ Partials       3747     2669     -1078

Flag	Coverage Δ
unittests	`34.23% <62.50%> (-15.83%)`	⬇️

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

Files with missing lines	Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java	`47.80% <100.00%> (-11.26%)`	⬇️
.../service/SystemTopicBasedTopicPoliciesService.java	`52.13% <60.86%> (-12.29%)`	⬇️

... and 278 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gaoran10

I'm not sure we need to handle the warning, it's expected behavior.

gaoran10 · 2022-12-08T09:07:35Z

        String namespace = tenant + "/test-ns2";
+        admin.namespaces().createNamespace(namespace, Set.of("test"));
+        admin.topics().createNonPartitionedTopic(namespace + "/tobedeleted");
+        // verify namespace can be deleted even without topic policy events


I'm not sure why the namespace couldn't be deleted when there is no topic policy events.

it's the scope of this pull 😁
When deleting namespace with force=true, you first delete the topics in it. For each topics deleted, a new event is generated. However if the __change_events doesn't exist yet, it's auto-created. But the creation fails since the namespace is being deleted.

btw I've added a more specific unit test case

nicoloboschi · 2022-12-10T08:11:58Z

@gaoran10 @Technoboy- PTAL

… been created yet (apache#18804)

… been created yet (#18804) (cherry picked from commit 789122b)

… been created yet (#18804)

[fix][broker] Fix namespace deletion if __change_events has not been …

56b34d6

…created yet

nicoloboschi requested review from liangyepianzhou and mattisonchao December 7, 2022 14:58

github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 7, 2022

nicoloboschi mentioned this pull request Dec 7, 2022

[run-tests] [fix][broker] Fix namespace deletion if __change_events topic has not been created yet nicoloboschi/pulsar#38

Closed

Technoboy- assigned nicoloboschi Dec 8, 2022

Technoboy- added this to the 2.12.0 milestone Dec 8, 2022

Technoboy- added area/broker ready-to-test labels Dec 8, 2022

Technoboy- closed this Dec 8, 2022

Technoboy- reopened this Dec 8, 2022

Technoboy- reviewed Dec 8, 2022

View reviewed changes

Comment thread ...ker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java Outdated

gaoran10 reviewed Dec 8, 2022

View reviewed changes

nicoloboschi added 2 commits December 9, 2022 10:22

make it async

68a7d23

rm double check about ns policies

c37169e

Technoboy- reviewed Dec 11, 2022

View reviewed changes

Comment thread ...ker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java

nicoloboschi added 2 commits December 12, 2022 14:40

simplify code

7facf37

style

db8cfaf

Technoboy- approved these changes Dec 12, 2022

View reviewed changes

fix flaky test

ac72e17

nicoloboschi added release/2.11.1 release/2.10.3 labels Dec 13, 2022

nicoloboschi merged commit 789122b into apache:master Dec 13, 2022

liangyepianzhou added release/2.10.4 and removed release/2.10.3 labels Dec 14, 2022

lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023

[fix][broker] Fix namespace deletion if __change_events topic has not…

158d497

… been created yet (apache#18804)

liangyepianzhou added release/2.10.5 and removed release/2.10.4 labels Feb 26, 2023

liangyepianzhou added the cherry-picked/branch-2.10 label Jul 8, 2023

liangyepianzhou pushed a commit that referenced this pull request Jul 8, 2023

[fix][broker] Fix namespace deletion if __change_events topic has not…

043dbc3

… been created yet (#18804) (cherry picked from commit 789122b)

liangyepianzhou added a commit that referenced this pull request Jul 8, 2023

[fix][test] Fix the test introduced in #18804

9a6a460

Technoboy- pushed a commit that referenced this pull request Jul 17, 2023

[fix][broker] Fix namespace deletion if __change_events topic has not…

721275a

… been created yet (#18804)

Technoboy- added cherry-picked/branch-2.11 release/2.11.3 and removed release/2.11.1 labels Jul 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[fix][broker] Fix namespace deletion if __change_events topic has not been created yet - #18804

[fix][broker] Fix namespace deletion if __change_events topic has not been created yet#18804
nicoloboschi merged 6 commits into
apache:masterfrom
nicoloboschi:fix-namespace-del

nicoloboschi commented Dec 7, 2022

Uh oh!

codecov-commenter commented Dec 8, 2022 •

edited

Loading

Uh oh!

Uh oh!

gaoran10 left a comment

Uh oh!

gaoran10 Dec 8, 2022

Uh oh!

nicoloboschi Dec 9, 2022

Uh oh!

nicoloboschi Dec 9, 2022

Uh oh!

Uh oh!

nicoloboschi commented Dec 10, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nicoloboschi commented Dec 7, 2022

Motivation

Modifications

Verifying this change

Documentation

Uh oh!

codecov-commenter commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

gaoran10 left a comment

Choose a reason for hiding this comment

Uh oh!

gaoran10 Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

nicoloboschi Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

nicoloboschi Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicoloboschi commented Dec 10, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Dec 8, 2022 •

edited

Loading