Skip to content

[fix][broker] Close pending acks cleanup gap in BacklogQuotaManager - #25624

Merged
nodece merged 1 commit into
apache:masterfrom
nodece:backlog-quota-manager-pending-acks-cleanup
Apr 30, 2026
Merged

[fix][broker] Close pending acks cleanup gap in BacklogQuotaManager#25624
nodece merged 1 commit into
apache:masterfrom
nodece:backlog-quota-manager-pending-acks-cleanup

Conversation

@nodece

@nodece nodece commented Apr 30, 2026

Copy link
Copy Markdown
Member

Fix #25613

Motivation

PR #25592 added Dispatcher#markDeletePositionMoveForward() to clean up pending acks and redelivery state when cursors advance via subscription-level APIs (skip, expiry, clear-backlog).

However, BacklogQuotaManager bypasses these APIs and directly updates ManagedCursor, so the hook is not triggered in two eviction paths:

  • Size eviction: ManagedCursor.skipEntries
  • Time eviction (non-precise): ManagedCursor.markDelete

This leads to stale state in Key_Shared subscriptions:

  • pendingAcks not cleaned up
  • unackedMessages drift

Modifications

  • BacklogQuotaManager
    • Add helper to trigger Dispatcher#markDeletePositionMoveForward
    • Invoke after cursor advancement in:
      • size eviction
      • time eviction (non-precise)
    • Extract eviction math into shared methods:
      • computeEvictionTarget
      • computeEntriesToEvict

@lhotari lhotari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@nodece
nodece merged commit d861e15 into apache:master Apr 30, 2026
81 of 83 checks passed
@nodece
nodece deleted the backlog-quota-manager-pending-acks-cleanup branch April 30, 2026 07:19
lhotari added a commit to lhotari/pulsar that referenced this pull request May 5, 2026
…409 cascade)

Two `BacklogQuotaManagerTest` cases consistently fail together: an
`@AfterMethod` `clearNamespaces` times out at 10s while force-deleting
`prop/ns-quota`, leaving the namespace in metadata; the next test's
`@BeforeMethod` `createNamespaces` then hits HTTP 409 ("Namespace already
exists") and the cascade continues for every subsequent test.

Root cause: PR apache#25624 added two pending-acks tests that leave Key_Shared
consumers with unacked messages on `prop/ns-quota` and an active
backlog-quota check loop running every 2s. Force-deleting that state can
exceed the 10s default `Awaitility` budget used by the shared
`MockedPulsarServiceBaseTest.deleteNamespaceWithRetry`. The afterMethod
times out, leftover metadata remains, and the next beforeMethod hits 409.

Fix:
1. `MockedPulsarServiceBaseTest.deleteNamespaceWithRetry` now uses
   `atMost(20, SECONDS)` instead of relying on the Awaitility 10s
   default. 2× headroom is enough for the heaviest cleanups in this
   class while staying conservative for the many tests that share this
   helper.
2. `BacklogQuotaManagerTest.createNamespaces` is split into a helper
   that catches `ConflictException` and recovers by force-deleting the
   leftover and recreating, so a slow prior cleanup cannot cascade as a
   409 — defense in depth on top of (1).

Verified locally with `@Test(invocationCount = 10)` on the two new
pending-acks tests: 20/20 passes, all `clearNamespaces` cycles within
the new 20s budget.
lhotari pushed a commit that referenced this pull request May 13, 2026
nodece added a commit to ascentstream/pulsar that referenced this pull request May 27, 2026
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 9, 2026
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.

Address remaining gap in pending acks cleanup for BacklogQuotaManager

3 participants