Skip to content

[fix][broker] Prevent early replay of non-strict delayed messages#26188

Merged
nodece merged 1 commit into
masterfrom
codex/fix-delayed-delivery-bucket-cutoff
Jul 14, 2026
Merged

[fix][broker] Prevent early replay of non-strict delayed messages#26188
nodece merged 1 commit into
masterfrom
codex/fix-delayed-delivery-bucket-cutoff

Conversation

@coderzc

@coderzc coderzc commented Jul 14, 2026

Copy link
Copy Markdown
Member

Motivation

InMemoryDelayedDeliveryTracker rounds non-strict delivery timestamps down to reduce the number of timestamp keys. Availability checks currently compare that bucket start directly with the non-strict cutoff (now + tickTimeMillis).

The original deliverAt can be up to precisionMillis - 1 later than the bucket start. As a result, the tracker can expose a position before the original timestamp is inside the configured early-delivery window. The dispatcher then reads the entry, addMessage checks the original deliverAt, and inserts the same position back into an already eligible bucket. An active dispatcher can repeat this replay until the original timestamp reaches the cutoff, causing unnecessary reads.

With the default 1000 ms tick and 512 ms timestamp buckets, this inconsistent window can be up to 511 ms.

Regression source and related changes

  • #23611 changed the in-memory tracker from a per-message priority queue to timestamp buckets and trimmed the lower timestamp bits. This is the change that introduced the timestamp-down-rounding behavior involved in this issue.
  • #26012 fixed the analogous early-bucket problem for strict delivery by changing strict-mode cutoff handling. This PR complements that fix for non-strict mode, where the configured early-delivery window must remain unchanged.

Modifications

  • Adjust the non-strict bucket cutoff by the maximum timestamp rounding distance, so a bucket is exposed only when its upper bound is inside the configured early-delivery window.
  • Keep the existing timestamp representation, strict-mode behavior, timer state machine, and configuration semantics unchanged.
  • Add a deterministic regression test that runs repeated dispatch rounds at a fixed clock time and counts replay reads of the same position.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • ./gradlew :pulsar-broker:test --tests 'org.apache.pulsar.broker.delayed.InMemoryDeliveryTrackerTest' -PtestRetryCount=0
  • ./gradlew quickCheck

The new regression test was also verified against the unpatched implementation: the same position was replayed and re-added 10 times while the clock remained fixed. With the fix it is replayed zero times before the early-delivery window, then exactly once after the full bucket enters that window.

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

@coderzc coderzc requested review from lhotari and nodece and removed request for lhotari July 14, 2026 07:35
@coderzc coderzc self-assigned this Jul 14, 2026
@coderzc coderzc added type/bug The PR fixed a bug or issue reported a bug area/broker release/4.2.4 release/4.0.13 labels Jul 14, 2026
@lhotari lhotari added this to the 5.0.0-M2 milestone Jul 14, 2026
@lhotari

lhotari commented Jul 14, 2026

Copy link
Copy Markdown
Member

#23611 was reverted from 4.0.x by #24429 so there's no need for fixing this in branch-4.0.

@nodece nodece merged commit 16476b5 into master Jul 14, 2026
85 of 87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker release/4.2.4 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants