-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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] Delete compacted ledger when topic is deleted #21745
Conversation
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Show resolved
Hide resolved
@@ -106,5 +108,16 @@ public void markDeleteFailed(ManagedLedgerException exception, Object ctx) { | |||
} | |||
} | |||
|
|||
CompletableFuture<Void> cleanCompactedLedger() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have TopicCompactionService
now, shall we add an new API to the TopicCompactionService
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is just to fix this old issue, it needs to be cherry-picked to the release branch, I think we can separately propose a PIP extending TopicCompactionService on the later version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sure.
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Show resolved
Hide resolved
ebe004a
to
c9d3068
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition, to keep the same style, you can use assertEquals
instead of Assert.assertEquals
.
pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactionTest.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactionTest.java
Outdated
Show resolved
Hide resolved
66ecc41
to
5f96de3
Compare
5f96de3
to
3a2bf54
Compare
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Show resolved
Hide resolved
(cherry picked from commit 23391d3)
Motivation
Currently, the compacted ledger doesn't delete when the topic is deleted, this will result in many orphaned ledgers, we should delete them.
Modifications
Delete compacted ledger when
__compaction
cursor is deleted.Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: