Skip to content

[fix][broker] Keep deduplication snapshots recoverable after skipped or failed attempts - #285

Open
lhotari wants to merge 6 commits into
masterfrom
lh-perfopt-dedup-snapshot-coalescing
Open

lhotari wants to merge 6 commits into
masterfrom
lh-perfopt-dedup-snapshot-coalescing

Conversation

@lhotari

@lhotari lhotari commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Motivation

Deduplication snapshots are asynchronous and intentionally allow only one cursor mark-delete at a time. At high publish rates, the entry threshold can expire several times before the current snapshot completes. This expected overlap currently emits a warning for every skipped attempt.

Two failure paths can also leave snapshotTaking set indefinitely. A publish completion can race with disabling deduplication and observe a null cursor after acquiring the guard, and asyncMarkDelete can throw synchronously before installing a callback. After either case, later snapshots are suppressed even after deduplication is enabled again.

Modifications

  • Treat overlapping snapshot triggers as expected coalescing and log them at DEBUG.
  • Capture and validate the managed cursor before acquiring the in-flight guard.
  • Convert synchronous snapshot initiation failures to failed futures.
  • Clear the guard from one completion path for both asynchronous success and failure.
  • Preserve the existing snapshot interval, entry-counter behavior, cursor properties, and replay failure propagation.
  • Add deterministic tests for overlap, asynchronous and synchronous failures, disable/re-enable races, stale cursors, and recovery retry behavior.

Validation

The change is based directly on current master and was verified with:

  • Seven focused snapshot tests.
  • BrokerMessageDeduplicationTest.
  • MessageDuplicationTest.
  • Full quickCheck.
  • An end-to-end saturation run with one million warmup messages followed by five million measured messages from 500 stable producers to one topic and five ten-consumer Key_Shared subscriptions. All 30 million fanout deliveries arrived with no missing, duplicate, invalid, or out-of-order messages.

The same workload was profiled with and without this patch:

Revision Measured ingress Measurement window Pending-snapshot warnings
parent 87,316 msg/s 57.263 s 3,414
candidate 83,438 msg/s 59.925 s 0

The candidate run experienced more CPU thermal throttling, and adjacent runs showed substantial cache and thermal variation. The table establishes that warning churn is removed while delivery remains correct; it is intentionally not presented as a throughput comparison. This change fixes snapshot recovery and removes misleading warnings rather than claiming a capacity improvement.

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

TomaszGaweda and others added 6 commits September 19, 2026 16:22
…n denial logs (apache#26642)

Apply the configured role logging anonymizer consistently to topic authorization denial logs.
…reateSchema (apache#26643)

Check topic permissions for binary GetSchema and GetOrCreateSchema requests and retain the schema and transaction tests when merging master.
…or failed attempts

Treat overlapping snapshot triggers as expected and release the in-flight guard on every acquired path. Preserve the existing snapshot interval and propagate recovery failures without interrupting successful publish completion.

Assisted-by: GPT-6 Astra
@lhotari
lhotari force-pushed the lh-perfopt-dedup-snapshot-coalescing branch from a315551 to ea144d0 Compare September 19, 2026 18:47
@lhotari
lhotari changed the base branch from lh-perfopt-dedup-snapshot-base to master September 19, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants