Skip to content

[fix][broker] Orphan schema after disabled a cluster for a namespace #24223

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

Merged
merged 3 commits into from
Apr 30, 2025

Conversation

poorbarcode
Copy link
Contributor

Motivation

Use case

  • Enabling replication with global ZK
  • Disable replication.
    • Disable replication for the namespace, set namespace's clusters to [cluster-a]
    • The topics under cluster b will be automatically removed since the namespace has no cluster named cluster-b
  • Delete topics
    • Delete topics under the namespace on cluster-a, since all topics were deleted on cluster-b automatically
    • Issue: the schema has not been removed under cluster-b

Modifications

  • Remove schema if all partitions were removed because the cluster has been removed

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode self-assigned this Apr 28, 2025
@poorbarcode poorbarcode added this to the 4.1.0 milestone Apr 28, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 28, 2025
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@lhotari lhotari changed the title [fix][broker]Orphan schema after disabled a cluster for a namespace [fix][broker] Orphan schema after disabled a cluster for a namespace Apr 28, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari
Copy link
Member

lhotari commented Apr 28, 2025

@poorbarcode checkstyle error

[INFO] There is 1 error reported by Checkstyle 10.14.2 with /home/runner/work/pulsar/pulsar/buildtools/src/main/resources/pulsar/checkstyle.xml ruleset.
Error:  src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:[1999] (sizes) LineLength: Line is longer than 120 characters (found 126).

@poorbarcode poorbarcode force-pushed the fix/replication_schemaId branch from 5ee7c00 to ee22625 Compare April 29, 2025 12:51
@poorbarcode
Copy link
Contributor Author

@poorbarcode checkstyle error

Fixed

@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2025

Codecov Report

Attention: Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.21%. Comparing base (bbc6224) to head (ee22625).
Report is 1057 commits behind head on master.

Files with missing lines Patch % Lines
...sar/broker/service/persistent/PersistentTopic.java 90.62% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24223      +/-   ##
============================================
+ Coverage     73.57%   74.21%   +0.64%     
+ Complexity    32624    32573      -51     
============================================
  Files          1877     1866      -11     
  Lines        139502   144874    +5372     
  Branches      15299    16548    +1249     
============================================
+ Hits         102638   107522    +4884     
+ Misses        28908    28854      -54     
- Partials       7956     8498     +542     
Flag Coverage Δ
inttests 26.67% <0.00%> (+2.09%) ⬆️
systests 23.30% <0.00%> (-1.02%) ⬇️
unittests 73.70% <90.62%> (+0.85%) ⬆️

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

Files with missing lines Coverage Δ
...sar/broker/service/persistent/PersistentTopic.java 80.00% <90.62%> (+1.54%) ⬆️

... and 1081 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.

@Technoboy- Technoboy- merged commit 2d78cbd into apache:master Apr 30, 2025
52 checks passed
lhotari pushed a commit that referenced this pull request Apr 30, 2025
lhotari pushed a commit that referenced this pull request Apr 30, 2025
lhotari pushed a commit that referenced this pull request Apr 30, 2025
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 6, 2025
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 7, 2025
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 7, 2025
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 12, 2025
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 28, 2025
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.

4 participants