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

[fix][broker] Check migration cluster exist when enable namespace migration in blue-green migration #22362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Mar 26, 2024

Motivation

#21367 Support to migrate topics from blue to green cluster per namespace

But when we enable namespace migration, there was no check if the migrate cluster existed. There will be some unusual situations here.

Minimal reproduce step:
1.Start a single broker node and create a tenant test-tenant
sh bin/pulsar-admin tenants create test-tenant

2.Create a new namespace and a topic.
sh bin/pulsar-admin namespaces create test-tenant/test-ns
sh bin/pulsar-admin topics create test-tenant/test-ns/test-topic

3.Enable namespaces migration
sh bin/pulsar-admin namespaces update-migration-state --migrated test-tenant/test-ns

We will see the following log:
image

Modifications

  1. Check migration cluster exist when enable namespace migration.
  2. Determine whether clusterData is exists and isNamespaceMigrationEnabled=true satisfied at the same time in method getMigratedClusterUrlAsync

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 26, 2024
@hanmz
Copy link
Contributor Author

hanmz commented Mar 26, 2024

Please help me review it when you have time. @vraulji567 @rdhabalia @eolivelli Thank you so much.

@hanmz
Copy link
Contributor Author

hanmz commented Apr 1, 2024

/pulsarbot rerun-failure-checks

@Technoboy- Technoboy- closed this Apr 2, 2024
@Technoboy- Technoboy- reopened this Apr 2, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 72.90%. Comparing base (bbc6224) to head (4bd2aa4).
Report is 111 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22362      +/-   ##
============================================
- Coverage     73.57%   72.90%   -0.68%     
+ Complexity    32624    32304     -320     
============================================
  Files          1877     1892      +15     
  Lines        139502   145784    +6282     
  Branches      15299    16776    +1477     
============================================
+ Hits         102638   106279    +3641     
- Misses        28908    31227    +2319     
- Partials       7956     8278     +322     
Flag Coverage Δ
inttests 27.94% <0.00%> (+3.36%) ⬆️
systests 25.30% <0.00%> (+0.97%) ⬆️
unittests 72.04% <83.33%> (-0.81%) ⬇️

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

Files Coverage Δ
...rg/apache/pulsar/broker/service/AbstractTopic.java 87.98% <100.00%> (ø)
...pache/pulsar/broker/admin/impl/NamespacesBase.java 74.83% <80.00%> (+1.75%) ⬆️

... and 175 files with indirect coverage changes

@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
@lhotari lhotari modified the milestones: 4.0.0, 4.1.0 Oct 11, 2024
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.

5 participants