Skip to content

[fix][broker] Prevent completing replicated snapshot before marker publish - #26119

Merged
lhotari merged 3 commits into
apache:masterfrom
Denovo1998:prevent_completing_replicated_snapshot_before_marker_publish
Jul 25, 2026
Merged

[fix][broker] Prevent completing replicated snapshot before marker publish#26119
lhotari merged 3 commits into
apache:masterfrom
Denovo1998:prevent_completing_replicated_snapshot_before_marker_publish

Conversation

@Denovo1998

Copy link
Copy Markdown
Contributor

Motivation

Replicated subscription snapshots are currently marked as completed immediately after publishing the final snapshot marker. However, marker publishing on persistent topics completes asynchronously, so the final marker may still fail after the snapshot has already been removed from the pending map and recorded as the last completed snapshot.

When this happens, the existing timeout-based retry path cannot recover because the snapshot is no longer pending.

Modifications

  • Add a marker publishing path that exposes the publish callback result as a CompletableFuture.
  • Complete replicated subscription snapshots only after the final snapshot marker is successfully published.
  • Keep the snapshot pending when final marker publishing fails, allowing the existing timeout retry logic to handle it.
  • Avoid completing snapshots that have already been removed from the pending map.
  • Add a regression test that simulates final marker publish failure and verifies the snapshot remains pending.
  • Update snapshot builder tests to use the new callback-aware marker publishing path.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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

If the box was checked, please highlight the changes

  • 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

@void-ptr974 void-ptr974 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR makes replicated subscription snapshot completion wait for the final snapshot marker publish result. If publishing the final marker fails, the snapshot remains pending so the existing timeout retry path can handle it.

It also guards concurrent timeout/completion races and adds tests for final marker publish failure and timeout cleanup behavior.

@Denovo1998

Copy link
Copy Markdown
Contributor Author

@lhotari
To ensure everything is in order, I have just reviewed it again.

I can find the merged PRs directly related to the replicated-subscription snapshot since last time, mainly #26234. However, they appear to be complementary.

It should be fine, just check after the CI passes.

@lhotari
lhotari merged commit bc0e8b5 into apache:master Jul 25, 2026
82 of 83 checks passed
lhotari pushed a commit that referenced this pull request Jul 25, 2026
lhotari pushed a commit that referenced this pull request Jul 25, 2026
@Denovo1998
Denovo1998 deleted the prevent_completing_replicated_snapshot_before_marker_publish branch July 25, 2026 23:51
sandeep-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2026
sandeep-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2026
sandeep-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2026
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Aug 28, 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.

3 participants