pulsar
pulsar copied to clipboard
[improve][admin] PIP-369 Introduce `unload` flag in `ns-isolation-policy set` call
Implementation PR for the PIP 369 "Flag based selective unload on changing ns-isolation-policy" #23116
Modifications
Added an unload-scope flag in the ns-isolation-policy set call to control the behavior of which namespaces to unload as part of the set call in a more granular manner.
As this PR is targetted for 3.0 LTS as well, keeping the changes backward compatible. i.e. the flag unload-scope is optional. Default value is all_matching which means all namespaces matching the namespaces regex being set in the policy set call.
There will be a followup PR which will be adding the following breaking changes:
- Making the default value of the flag to be
changedi.e. only the newly added or now-removed namespace regex would be used to unload the namespaces - The meaning of
all_matchingwill change to cover all namespaces matching both the previous and the new namespace regex values. The idea is that if immediate placement correction is needed, we should also unload the namespaces which do not belong to the isolation group anymore.
Verifying this change
This change added tests and can be verified as follows:
- Added and made changes to integration tests covering this feature. All three unload scopes are tried and validation is done that only the relevant namespaces are unloaded
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
- [x] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
Documentation
- [ ]
doc - [x]
doc-required - [ ]
doc-not-needed - [ ]
doc-complete
PR in forked repo - https://github.com/grssam/pulsar/pull/1
@grssam Please add the following content to your PR description and select a checkbox:
- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
Please add labels for 3.0.7 release as I would like this to be present in the LTS version.
Not able to rerun the failed CI pipeline, but all tests passed in my repo - https://github.com/grssam/pulsar/pull/1 and the new tests that I added are part of "Broker Group 3" only.
Not able to rerun the failed CI pipeline, but all tests passed in my repo - https://github.com/grssam/pulsar/pull/1 and the new tests that I added are part of "Broker Group 3" only.
@grssam Pulsar CI, you can trigger a rerun by adding a comment "/pulsarbot rerun-failure-checks" to this PR. It requires that a possible previous workflow run has completed.
/pulsarbot rerun-failure-checks
/pulsarbot rerun-failure-checks
/pulsarbot rerun-failure-checks
Codecov Report
Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.
Project coverage is 74.55%. Comparing base (
bbc6224) to head (746f072). Report is 603 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #23120 +/- ##
============================================
+ Coverage 73.57% 74.55% +0.98%
- Complexity 32624 33656 +1032
============================================
Files 1877 1925 +48
Lines 139502 144951 +5449
Branches 15299 15849 +550
============================================
+ Hits 102638 108068 +5430
+ Misses 28908 28618 -290
- Partials 7956 8265 +309
| Flag | Coverage Δ | |
|---|---|---|
| inttests | 27.85% <10.81%> (+3.26%) |
:arrow_up: |
| systests | 24.65% <10.81%> (+0.33%) |
:arrow_up: |
| unittests | 73.90% <83.78%> (+1.05%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files with missing lines | Coverage Δ | |
|---|---|---|
| .../apache/pulsar/broker/admin/impl/ClustersBase.java | 81.76% <100.00%> (+1.26%) |
:arrow_up: |
| ...r/common/policies/data/NamespaceIsolationData.java | 100.00% <ø> (ø) |
|
| ...mmon/policies/data/NamespaceIsolationDataImpl.java | 89.18% <100.00%> (+3.89%) |
:arrow_up: |
| .../pulsar/admin/cli/CmdNamespaceIsolationPolicy.java | 37.83% <0.00%> (+1.25%) |
:arrow_up: |
| ...on/policies/impl/NamespaceIsolationPolicyImpl.java | 84.90% <50.00%> (-1.37%) |
:arrow_down: |
| ...cies/data/NamespaceIsolationPolicyUnloadScope.java | 50.00% <50.00%> (ø) |
PS: while cherry picking to branch-3.0 , it will contain one conflict due to the command line argument parser library change at line https://github.com/apache/pulsar/pull/23120/files#diff-63a2a931cc53a14767f5eb4a53898d7b6fb20f81a87de51cc15bbc139782bd00R77 (and the relevant class imports as well)
PS: while cherry picking to branch-3.0 , it will contain one conflict due to the command line argument parser library change at line https://github.com/apache/pulsar/pull/23120/files#diff-63a2a931cc53a14767f5eb4a53898d7b6fb20f81a87de51cc15bbc139782bd00R77 (and the relevant class imports as well)
@grssam Yes, that's common that there are conflicts. The committer performing the cherry-picking and backporting will request a separate PR for a backport to branch-3.0 if there's a need. Some of this is explained in the mailing list message https://lists.apache.org/thread/ryyfv8o33yyw9hmo5gnxn71boocn3mzs .
/pulsarbot rerun-failure-checks
merging...