Skip to content

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

Open
Denovo1998 wants to merge 2 commits into
apache:masterfrom
Denovo1998:prevent_completing_replicated_snapshot_before_marker_publish
Open

[fix][broker] Prevent completing replicated snapshot before marker publish#26119
Denovo1998 wants to merge 2 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.

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.

2 participants