Skip to content

[improve][client] Best-effort retry for individual/batch-index acks on send failure when ackReceiptEnabled=false - #25525

Merged
nodece merged 3 commits into
apache:masterfrom
nodece:improve-client-ack-send-failures
Apr 24, 2026
Merged

[improve][client] Best-effort retry for individual/batch-index acks on send failure when ackReceiptEnabled=false#25525
nodece merged 3 commits into
apache:masterfrom
nodece:improve-client-ack-send-failures

Conversation

@nodece

@nodece nodece commented Apr 15, 2026

Copy link
Copy Markdown
Member

Motivation

When ackReceiptEnabled=false, grouped consumer acks are sent in the background and local write failures could drop pending ack state. This increases the risk of ack holes that can block progress in cases like Key_Shared consumption.

Modifications

  • Retain pending grouped acks for best-effort retry when send fails and ackReceiptEnabled=false — applies to individual and batch-index acknowledgements only
  • Skip cumulative ack restore on send failure — cumulative acks cover unbounded ranges and restoring them after a partial send could re-ack already-processed messages, so retry is intentionally excluded
  • Keep ackReceiptEnabled=true on explicit future/ack-receipt failure semantics instead of internal pending restore
  • Add coverage for both no-receipt retry behavior and ack-receipt failure behavior

nodece and others added 3 commits April 15, 2026 17:22
…ReceiptEnabled=false

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@dao-jun dao-jun 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

@Denovo1998

Copy link
Copy Markdown
Contributor

The final patch no longer restores cumulative grouped acknowledgements on local write failure, making the behavior a best-effort retry for only individual and batch-index acknowledgements. If this change is intentional, could we update the PR title and body accordingly? Otherwise, this branch still loses the retry opportunity for grouped cumulative acknowledgements when ackReceiptEnabled=false.

@nodece nodece changed the title [improve][client] Best-effort retry for acks on send failure when ackReceiptEnabled=false [improve][client] Best-effort retry for individual/batch-index acks on send failure when ackReceiptEnabled=false Apr 24, 2026
@nodece

nodece commented Apr 24, 2026

Copy link
Copy Markdown
Member Author

@Denovo1998 Both the title and body have been updated. We only retry for individual ack when ackReceiptEnabled=false.

@nodece
nodece merged commit fae3df9 into apache:master Apr 24, 2026
44 checks passed
@nodece
nodece deleted the improve-client-ack-send-failures branch April 24, 2026 02:53
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request May 6, 2026
…n send failure when ackReceiptEnabled=false (apache#25525)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lhotari lhotari added this to the 5.0.0-M1 milestone Jun 1, 2026
lhotari pushed a commit that referenced this pull request Jun 1, 2026
…n send failure when ackReceiptEnabled=false (#25525)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit fae3df9)
lhotari pushed a commit that referenced this pull request Jun 1, 2026
…n send failure when ackReceiptEnabled=false (#25525)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit fae3df9)
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 9, 2026
…n send failure when ackReceiptEnabled=false (apache#25525)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit fae3df9)
(cherry picked from commit bb0b7d5)
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.

4 participants