[fix][broker]Fix deadlock when compaction and topic deletion execute concurrently #24366

poorbarcode · 2025-05-30T08:00:48Z

Motivation

Issue 1: deadlock when compaction and topic deletion execute concurrently

Reproduction steps of the deadlock( see also the new test testConcurrentCompactionAndTopicDelete )

thread-compaction: create a raw reader.
thread-topic-deleting: mark the topic as deleted and disconnect clients, including the compaction reader.
thread-compaction: The raw reader attempts to reconnect due to the disconnection in step "2-1".
thread-topic-deleting: unsubscribe all subscriptions that contain "__compaction".
thread-topic-deleting: deleting "__compaction" cursor will wait for the in-progress compaction task.
thread-compaction: the raw read can not connect successfully anymore because the topic was marked as "fenced && isDeleting".
deadlock: "thread-topic-deleting" waiting for compaction task being finished, thread-compaction continuously reconnects.

Note:

If the thread-compaction is stuck when the first phase of compaction calling reader.readNextAsync() at step 6, the deadlock can be solved after {brokerServiceCompactionPhaseOneLoopTimeInSeconds) seconds
But regarding other scenarios, such as reader.getLastMessageIdAsync... the deadlock will persist forever.

You can reproduce the issue by the new test testConcurrentCompactionAndTopicDelete

Issue 2-1: The responded compostable future of `Consumer.receiveAsync()` never complete if the topic is deleted

Background:

Enabling topic auto creation: Client triggers a new topic creation if users delete a topic with -f.
Disabling topic auto creation: Client stops retrying reconnection and sets the consumer's state to Failed.
Issue: regarding the scenario "Disabling topic-auto-creation", the client set the consumer's state to Failed, but it does not trigger a completion for the pending receive requests, leading to the pending receive future never completing. See https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java#L988-L999

setState(State.Failed);
closeConsumerTasks();
client.cleanupConsumer(this);
// Issue: it should call "failPendingReceive()" also

You can reproduce the issue by the new test testReceiveWillDoneAfterTopicDeleted

Issue 2-2: resource leak that contains a dead letter producer and a retry letter producer.

Regarding issue 2-1, the client also forgot to release the related dead letter producer and the retry letter producer, which also caused a resource leak. Since there are other issues that prevent the new test from passing, I will write a separate PR to add the test to cover this case, which also contains another fix.

Issue 3: Even though the client received a `TopicNotFound` error, it continuously retries.

Root cause: Pulsar client prevents retrying when it receives a TopicNotFound error from the Subscribe command, but the TopicNotFound can also be received from the process get broker connections of the target topic.

You can reproduce the issue by the new test testReceiveWillDoneAfterTopicDeleted

Modifications

Regarding issue 1 - a deadlock error, closes the RawReader if the compaction task received a ServiceNotReady error, which happens when a topic/subscription is fenced, or the topic/namespace is loading/unloading. Since the RawReader is an internal API, this change is safe.
Regarding issue 2 - consumer forgets to complete the pending read request when it stops reconnecting, calls consumer.close() instead of calling a part of the stats modifying, which is more complete.
Regarding issue 3 - continues to reconnect even though it received a TopicNotFound error, stops reconnecting after receiving an unrecovered error from grabbing new connections, and closes consumers/producers.

Documentation

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

Matching PR in forked repository

PR in forked repository: x

…concurrently

poorbarcode · 2025-05-30T08:09:13Z

/pulsarbot rerun-failure-checks

codecov-commenter · 2025-05-30T09:33:43Z

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (bbc6224) to head (fa2f120).
⚠️ Report is 1256 commits behind head on master.

Files with missing lines	Patch %	Lines
...va/org/apache/pulsar/client/impl/ConsumerImpl.java	76.19%	3 Missing and 2 partials ⚠️
...va/org/apache/pulsar/client/impl/ProducerImpl.java	83.33%	1 Missing and 2 partials ⚠️
...a/org/apache/pulsar/client/impl/RawReaderImpl.java	88.88%	0 Missing and 1 partial ⚠️
...g/apache/pulsar/client/impl/ConnectionHandler.java	85.71%	1 Missing ⚠️
...rg/apache/pulsar/client/impl/TopicListWatcher.java	50.00%	1 Missing ⚠️
...ulsar/client/impl/TransactionMetaStoreHandler.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #24366      +/-   ##
============================================
+ Coverage     73.57%   74.26%   +0.68%     
- Complexity    32624    32692      +68     
============================================
  Files          1877     1867      -10     
  Lines        139502   145260    +5758     
  Branches      15299    16610    +1311     
============================================
+ Hits         102638   107876    +5238     
+ Misses        28908    28853      -55     
- Partials       7956     8531     +575

Flag	Coverage Δ
inttests	`26.64% <44.61%> (+2.05%)`	⬆️
systests	`23.27% <9.23%> (-1.06%)`	⬇️
unittests	`73.76% <81.53%> (+0.91%)`	⬆️

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/AbstractTopic.java	`88.04% <ø> (+0.06%)`	⬆️
...sar/broker/service/persistent/PersistentTopic.java	`80.24% <100.00%> (+1.78%)`	⬆️
...n/java/org/apache/pulsar/client/api/RawReader.java	`100.00% <100.00%> (ø)`
...n/java/org/apache/pulsar/compaction/Compactor.java	`92.30% <100.00%> (+11.53%)`	⬆️
...pache/pulsar/client/api/PulsarClientException.java	`64.40% <100.00%> (+1.35%)`	⬆️
.../java/org/apache/pulsar/client/impl/ClientCnx.java	`70.12% <100.00%> (-1.66%)`	⬇️
...a/org/apache/pulsar/client/impl/RawReaderImpl.java	`84.07% <88.88%> (-0.70%)`	⬇️
...g/apache/pulsar/client/impl/ConnectionHandler.java	`86.17% <85.71%> (-0.60%)`	⬇️
...rg/apache/pulsar/client/impl/TopicListWatcher.java	`65.69% <50.00%> (-2.17%)`	⬇️
...ulsar/client/impl/TransactionMetaStoreHandler.java	`67.66% <50.00%> (-0.24%)`	⬇️
... and 2 more

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

lhotari · 2025-05-30T12:10:14Z

@poorbarcode Does this resolve #24148?

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

lhotari · 2025-05-30T13:09:15Z

why does the current code (before this PR) handle the if (isClosingOrDeleting) { check inside the write lock?

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

Lines 1478 to 1483 in 81c94c8

    
           lock.writeLock().lock(); 
        
           try { 
        
               if (isClosingOrDeleting) { 
        
                   log.warn("[{}] Topic is already being closed or deleted", topic); 
        
                   return FutureUtil.failedFuture(new TopicFencedException("Topic is already fenced")); 
        
               }

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

codelipenghui · 2025-05-30T17:07:16Z

deadlock: "thread-topic-deleting" waiting for compaction task being finished, thread-compaction continuously reconnects.

Actually, we are safe to give up the reconnect from the compaction reader if the topic is deleting.

Now, the solution is to unfence the topic if there is ongoing compaction task. But it unfencing will also make producers/consumers from user side available to connect to this topic which should not be expected.

So, I think the correct fix should be find a way to expose the "Topic is deleting Exception" to Raw reader and the reader can optionally give up the retry.

poorbarcode · 2025-06-01T06:41:43Z

@codelipenghui

Actually, we are safe to give up the reconnect from the compaction reader if the topic is deleting.
Now, the solution is to unfence the topic if there is ongoing compaction task. But it unfencing will also make producers/consumers from user side available to connect to this topic which should not be expected.
So, I think the correct fix should be find a way to expose the "Topic is deleting Exception" to Raw reader and the reader can optionally give up the retry.

The solution you mentioned is what PIP-387: Modify interface TopicCompactionService to support cancelling an in-progress compaction task, which has been canceled 😂

gaoran10 · 2025-06-03T03:16:11Z

We'd better make the compaction task aware of the topic deletion operation and cancel the task.

poorbarcode · 2025-06-04T13:56:27Z

@codelipenghui @gaoran10 @lhotari I have used a new solution to solve the issue: cancel the pending compaction when deleting the topic, please review again

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TopicListWatcher.java

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

pulsar-broker/src/main/java/org/apache/pulsar/client/impl/RawReaderImpl.java

pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactionTest.java

lhotari · 2025-06-09T13:50:07Z

@poorbarcode Please handle cherry-picking to branch-3.0 and branch-3.3 since there are merge conflicts.

…concurrently (#24366) (cherry picked from commit 37e160f)

Date: Thu Jun 5 10:02:53 2025 +0800 [fix][broker]Fix deadlock when compaction and topic deletion execute concurrently (#24366) (cherry picked from commit 37e160f)

…concurrently (#24366) (cherry picked from commit 37e160f)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e) (cherry picked from commit ec2037e)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e) (cherry picked from commit ec2037e)

…concurrently (apache#24366) (cherry picked from commit 37e160f)

lhotari · 2025-06-19T10:53:49Z

@poorbarcode Please handle cherry-picking to branch-3.0 and branch-3.3 since there are merge conflicts.

@poorbarcode branch-3.0 compilation fails in checkstyle.

[INFO] There are 5 errors reported by Checkstyle 10.14.2 with ../buildtools/src/main/resources/pulsar/checkstyle.xml ruleset.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[53,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.api.SubscriptionInitialPosition.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[54,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.api.SubscriptionType.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[55,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.impl.conf.ConsumerConfigurationData.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[56,8] (imports) UnusedImports: Unused import: org.apache.pulsar.common.api.proto.BrokerEntryMetadata.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[70,15] (imports) UnusedImports: Unused import: org.apache.pulsar.client.impl.RawReaderImpl.DEFAULT_RECEIVER_QUEUE_SIZE.

After cherry-picking, please make sure that checkstyle passes and the project builds.
A quick way to validate is to run this command:

mvn -Pcore-modules,-main -T 1C clean install -DskipTests -Dspotbugs.skip=true -DnarPluginPhase=none

lhotari · 2025-06-19T16:46:36Z

@poorbarcode Please handle cherry-picking to branch-3.0 and branch-3.3 since there are merge conflicts.

@poorbarcode branch-3.0 compilation fails in checkstyle.

[INFO] There are 5 errors reported by Checkstyle 10.14.2 with ../buildtools/src/main/resources/pulsar/checkstyle.xml ruleset.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[53,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.api.SubscriptionInitialPosition.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[54,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.api.SubscriptionType.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[55,8] (imports) UnusedImports: Unused import: org.apache.pulsar.client.impl.conf.ConsumerConfigurationData.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[56,8] (imports) UnusedImports: Unused import: org.apache.pulsar.common.api.proto.BrokerEntryMetadata.
[ERROR] src/test/java/org/apache/pulsar/client/impl/RawReaderTest.java:[70,15] (imports) UnusedImports: Unused import: org.apache.pulsar.client.impl.RawReaderImpl.DEFAULT_RECEIVER_QUEUE_SIZE.

After cherry-picking, please make sure that checkstyle passes and the project builds. A quick way to validate is to run this command:

mvn -Pcore-modules,-main -T 1C clean install -DskipTests -Dspotbugs.skip=true -DnarPluginPhase=none

@poorbarcode I fixed it in b92af789

…concurrently (apache#24366)

[fix][broker]Fix deadlock when compaction and topic deletion execute …

89a831c

…concurrently

poorbarcode added this to the 4.1.0 milestone May 30, 2025

poorbarcode self-assigned this May 30, 2025

poorbarcode added type/bug The PR fixed a bug or issue reported a bug release/3.0.13 release/4.0.6 release/3.3.8 labels May 30, 2025

github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 30, 2025

poorbarcode added the ready-to-test label May 30, 2025

lhotari mentioned this pull request May 30, 2025

[Bug] Compaction of __change_events topic is blocking forceful namespace/topic deletion #24148

Open

3 tasks

lhotari reviewed May 30, 2025

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java Outdated Show resolved Hide resolved

lhotari reviewed May 30, 2025

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java Outdated Show resolved Hide resolved

poorbarcode requested a review from lhotari June 3, 2025 14:27

use a new solution to solve the issue

bce9e0a

lhotari reviewed Jun 4, 2025

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java Show resolved Hide resolved

lhotari reviewed Jun 4, 2025

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TopicListWatcher.java Show resolved Hide resolved

lhotari reviewed Jun 4, 2025

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java Show resolved Hide resolved

poorbarcode added 2 commits June 4, 2025 22:51

fix issue

f0fe5d9

address comment

b32d963

poorbarcode requested a review from lhotari June 4, 2025 15:09

codelipenghui reviewed Jun 4, 2025

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/client/impl/RawReaderImpl.java Show resolved Hide resolved

pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactionTest.java Show resolved Hide resolved

address comments

fa2f120

poorbarcode requested a review from codelipenghui June 4, 2025 17:17

codelipenghui approved these changes Jun 4, 2025

View reviewed changes

lhotari approved these changes Jun 4, 2025

View reviewed changes

poorbarcode merged commit 37e160f into apache:master Jun 5, 2025
53 checks passed

lhotari pushed a commit that referenced this pull request Jun 9, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

92a1930

…concurrently (#24366) (cherry picked from commit 37e160f)

lhotari added the cherry-picked/branch-4.0 label Jun 9, 2025

poorbarcode added a commit that referenced this pull request Jun 10, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

cae803e

…concurrently (#24366) (cherry picked from commit 37e160f)

poorbarcode added cherry-picked/branch-3.0 cherry-picked/branch-3.3 labels Jun 10, 2025

poorbarcode added a commit that referenced this pull request Jun 10, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

aa1bd9c

…concurrently (#24366) (cherry picked from commit 37e160f)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 12, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

f358076

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 12, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

ec2037e

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 12, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

c74a509

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

ganesh-ctds mentioned this pull request Jun 12, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute … datastax/pulsar#432

Closed

4 tasks

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 12, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

f1f1b6e

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 13, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

cb1a319

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 13, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

7a70d2b

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 13, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

c95af47

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit cae803e)

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 16, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

209aa34

…concurrently (apache#24366) (cherry picked from commit 37e160f) (cherry picked from commit 92a1930)

nodece pushed a commit to nodece/pulsar that referenced this pull request Jun 18, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

6212fa3

…concurrently (apache#24366) (cherry picked from commit 37e160f)

KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

16ecbed

…concurrently (apache#24366)

walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025

[fix][broker]Fix deadlock when compaction and topic deletion execute …

a90e603

…concurrently (apache#24366)

[fix][broker]Fix deadlock when compaction and topic deletion execute concurrently #24366

[fix][broker]Fix deadlock when compaction and topic deletion execute concurrently #24366

Uh oh!

Conversation

poorbarcode commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Issue 1: deadlock when compaction and topic deletion execute concurrently

Issue 2-1: The responded compostable future of Consumer.receiveAsync() never complete if the topic is deleted

Issue 2-2: resource leak that contains a dead letter producer and a retry letter producer.

Issue 3: Even though the client received a TopicNotFound error, it continuously retries.

Modifications

Documentation

Matching PR in forked repository

Uh oh!

poorbarcode commented May 30, 2025

Uh oh!

codecov-commenter commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lhotari commented May 30, 2025

Uh oh!

Uh oh!

lhotari commented May 30, 2025

Uh oh!

Uh oh!

codelipenghui commented May 30, 2025

Uh oh!

poorbarcode commented Jun 1, 2025

Uh oh!

gaoran10 commented Jun 3, 2025

Uh oh!

poorbarcode commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhotari commented Jun 9, 2025

Uh oh!

lhotari commented Jun 19, 2025

Uh oh!

lhotari commented Jun 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

poorbarcode commented May 30, 2025 •

edited

Loading

Issue 2-1: The responded compostable future of `Consumer.receiveAsync()` never complete if the topic is deleted

Issue 3: Even though the client received a `TopicNotFound` error, it continuously retries.

codecov-commenter commented May 30, 2025 •

edited

Loading