Skip to content

[fix][client] Fix unAckedMessageTracker cleanup on multi-topics batch ack - #26001

Merged
lhotari merged 3 commits into
apache:masterfrom
Dream95:fix_multi-topics-consumer-batch-ack-tracker
Jul 25, 2026
Merged

lhotari merged 3 commits into
apache:masterfrom
Dream95:fix_multi-topics-consumer-batch-ack-tracker

Conversation

@Dream95

@Dream95 Dream95 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Motivation

This fixes a bug in MultiTopicsConsumerImpl.doAcknowledge(List<MessageId>, ...).
When batch acknowledging messages across multiple topics, the success callback used
messageIdList.forEach(unAckedMessageTracker::remove) instead of
messageIds.forEach(unAckedMessageTracker::remove). As a result, after one underlying
consumer ack succeeded, all messages in the batch were removed from
unAckedMessageTracker, including messages that had not yet been acked by other consumers.

Modifications

  • In MultiTopicsConsumerImpl.doAcknowledge(List<MessageId>, ...), remove only the per-consumer
    messageIds from unAckedMessageTracker after a successful ack.
  • Add testBatchAcknowledgeRemovesOnlyAckedMessageIdsFromTracker to verify that completing one
    consumer's batch ack leaves the other consumer's message in the tracker.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

./gradlew :pulsar-client-original:test --tests org.apache.pulsar.client.impl.MultiTopicsConsumerImplTest.testBatchAcknowledgeRemovesOnlyAckedMessageIdsFromTracker

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

@lhotari lhotari 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

@lhotari lhotari added this to the 5.0.0-M1 milestone Jun 12, 2026
@lhotari lhotari modified the milestones: 5.0.0-M1, 5.0.0-M2 Jun 12, 2026
@Dream95

Dream95 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

The previous CI failure has been fixed on master. @lhotari Please approve the pending workflows. Thanks!

@Dream95
Dream95 requested a review from lhotari June 23, 2026 12:59
@lhotari
lhotari merged commit 3ebf796 into apache:master Jul 25, 2026
43 checks passed
lhotari pushed a commit that referenced this pull request Jul 26, 2026
lhotari pushed a commit that referenced this pull request Jul 26, 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
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.

4 participants