Skip to content

Conversation

@saosir
Copy link
Contributor

@saosir saosir commented Jan 11, 2021

Motivation

It is necessary to reset batchIndex of MessageId when tracking a message. Because only the ack of the last message in batch is
removed from UnAckedMessageTrackerEnabled

Verifying this change

add test case: testBatchUnAckedMessageTracker()

Modifications

when track or untrack a message, reset batchIndex of MessageId to -1

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@saosir
Copy link
Contributor Author

saosir commented Jan 11, 2021

see #9072 (comment)

@saosir
Copy link
Contributor Author

saosir commented Jan 11, 2021

/pulsarbot run-failure-checks

1 similar comment
@jiazhai
Copy link
Member

jiazhai commented Jan 11, 2021

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Jan 11, 2021

@BewareMyPower Would you please also take a look of this PR?

@sijie sijie added this to the 2.8.0 milestone Jan 11, 2021
@codelipenghui codelipenghui merged commit d4c1677 into apache:master Jan 12, 2021
@saosir saosir deleted the patch-6 branch January 12, 2021 01:23
merlimat pushed a commit to merlimat/pulsar that referenced this pull request Apr 6, 2021
…#9170)

### Motivation

 It is necessary to reset  batchIndex of MessageId when tracking a message. Because only the ack of the last message in batch  is 
 removed from `UnAckedMessageTrackerEnabled`

### Verifying this change
add test case: testBatchUnAckedMessageTracker()

### Modifications

when track or untrack a message, reset batchIndex of MessageId to -1
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