Skip to content

Conversation

@poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Feb 13, 2025

Motivation

Background

  • The steps of unsubscribing __compaction
    • Delete compacted ledger
    • Delete cursor
    • Remove subscription

Issue 1: consumer will stuck if deleting cursor(the step 2 above) failed

  • Delete compacted ledger
  • Delete cursor failed
  • Reload the topic
  • Compactor subscription relates to a deleted ledger
  • Consumers are stuck because the compacted ledger has been deleted, and the messages were lost. The broker will keep printing the error log below
2025-02-13T17:23:51,448 - ERROR - [broker-topic-workers-OrderedExecutor-4-0:PersistentDispatcherSingleActiveConsumer] - [persistent://public/default/tp-74ea581c-d7ca-4558-999b-804b6e33faee / reader-801f712e61-Consumer{subscription=PersistentSubscription{topic=persistent://public/default/tp-74ea581c-d7ca-4558-999b-804b6e33faee, name=reader-801f712e61}, consumerId=1, consumerName=Ti6va, address=[id: 0xce1b338f, L:/127.0.0.1:52193 - R:/127.0.0.1:52199] [SR:127.0.0.1, state:Connected]}] Error reading entries at 11:0 : org.apache.bookkeeper.client.BKException$BKNoSuchLedgerExistsException: No such ledger exists on Bookies - Retrying to read in 15.0 seconds

You can reproduce the issue by the test testReadMsgsAfterDisableCompaction(true)


issue 2
The compaction task can concurrently execute by deleting the cursor __compaction

Modifications

  • Fix issue 1
  • Fix issue 2
  • Print an error log if a compactor subscription is initialized twice, which may cause a ledger lost/

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode self-assigned this Feb 13, 2025
@poorbarcode poorbarcode added the type/bug The PR fixed a bug or issue reported a bug label Feb 13, 2025
@poorbarcode poorbarcode added this to the 4.1.0 milestone Feb 13, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 13, 2025
@lhotari lhotari changed the title [fix][broker]Consumer stuck when delete subscription __compaction failed [fix][broker] Consumer stuck when delete subscription __compaction failed Feb 13, 2025
@lhotari
Copy link
Member

lhotari commented Apr 9, 2025

@poorbarcode I fixed a checkstyle issue and a problem in using MockZooKeeper's delay method. After #23988, MockZooKeeper is correctly single threaded and a delay will block the execution of this code in ZKSessionWatcher:

try {
zkClientState = future.get(tickTimeMillis, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
// Consider zk disconnection if zk operation takes more than TICK_TIME
zkClientState = Watcher.Event.KeeperState.Disconnected;
}
checkState(zkClientState);

This will trigger a ConnectionLost session event. That's why a delay should be kept under 2000 ms.
Currently MockZooKeeper's session timeout is hard coded to 30000ms and the check interval is 1/15 of this, therefore 2000ms.

@lhotari
Copy link
Member

lhotari commented Apr 10, 2025

I created a separate PR to fix the invalid test that fails now: #24166

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. Good work @poorbarcode

@lhotari lhotari merged commit 98c9983 into apache:master Apr 10, 2025
51 of 52 checks passed
Technoboy- pushed a commit that referenced this pull request Apr 14, 2025
…iled (#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
poorbarcode added a commit to poorbarcode/pulsar that referenced this pull request Apr 15, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
lhotari added a commit that referenced this pull request Apr 17, 2025
…iled (#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
lhotari added a commit that referenced this pull request Apr 17, 2025
…iled (#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
nodece pushed a commit to nodece/pulsar that referenced this pull request Apr 22, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 24, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit e097ea1)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 24, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit e097ea1)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 27, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit e097ea1)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 30, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit e097ea1)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit e097ea1)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request May 2, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit 13d21f0)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 6, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
(cherry picked from commit 98c9983)
(cherry picked from commit 13d21f0)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
…iled (apache#23980)

Co-authored-by: Lari Hotari <lhotari@apache.org>
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