Skip to content

Conversation

@congbobo184
Copy link
Contributor

@congbobo184 congbobo184 commented Feb 16, 2022

Motivation

If individual ack batch message with transaction and abort this transaction, we will redeliver this message. but this batch message some bit sit are acked by another transaction and re consume this bit sit will produce TransactionConflictException, we don't need to redeliver this bit sit witch is acked by another transaction.

if batch have batch size 5

  1. txn1 ack 0, 1 the ackSet is 00111
  2. txn2 ack 2 3 4 the ack Set is 11000
  3. abort txn2 redeliver this position is 00111
  4. but now we don't filter txn1 ackSet so redeliver this position bitSet is 111111

Modifications

When filter the message we should filter the bit sit witch is real ack or in pendingAck state

Verifying this change

add the test

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? (yes)

  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

  • If a feature is not applicable for documentation, explain why?

  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

  • doc-not-needed

@github-actions
Copy link

@congbobo184:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions
Copy link

@congbobo184:Thanks for providing doc info!

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

congbo added 3 commits April 2, 2022 09:45
@congbobo184 congbobo184 changed the title [Transaction] Fix individual ack batch message with transaction abort redevlier duplicate messages [feature][txn] Fix individual ack batch message with transaction abort redevlier duplicate messages Apr 18, 2022
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.12.0 Jul 26, 2022
@Jason918
Copy link
Contributor

@congbobo184 Should we close this PR?

@congbobo184
Copy link
Contributor Author

@Jason918 don't close this PR, I will deal with it

@congbobo184 congbobo184 merged commit e0c0d5e into apache:master Feb 17, 2023
@thetumbled
Copy link
Member

Isn't there a problem of message loss with this PR?

@codelipenghui
Copy link
Contributor

We should carefully to cherry-pick this fix to release branch.

@mattisonchao
Copy link
Member

mattisonchao commented Feb 20, 2023

@thetumbled I will do testing to check the problem you mentioned.

liangyepianzhou pushed a commit that referenced this pull request Feb 25, 2023
…t redevlier duplicate messages (#14327)

If individual ack batch message with transaction and abort this transaction, we will redeliver this message. but this batch message some bit sit are acked by another transaction and re consume this bit sit will produce `TransactionConflictException`, we don't need to redeliver this bit sit witch is acked by another transaction.

if batch have batch size 5

1. txn1 ack 0, 1     the ackSet is   00111
2. txn2 ack 2 3 4 the ack Set is  11000
3. abort txn2 redeliver this position is 00111
4. but now we don't filter txn1 ackSet so redeliver this position bitSet is 111111
When filter the message we should filter the bit sit witch is real ack or in pendingAck state
add the test

(cherry picked from commit e0c0d5e)
@coderzc
Copy link
Member

coderzc commented Feb 27, 2023

@congbobo184 Can you help cherry-pick it to branch 2.9?

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Feb 28, 2023
…t redevlier duplicate messages (apache#14327)

If individual ack batch message with transaction and abort this transaction, we will redeliver this message. but this batch message some bit sit are acked by another transaction and re consume this bit sit will produce `TransactionConflictException`, we don't need to redeliver this bit sit witch is acked by another transaction.

if batch have batch size 5

1. txn1 ack 0, 1     the ackSet is   00111
2. txn2 ack 2 3 4 the ack Set is  11000
3. abort txn2 redeliver this position is 00111
4. but now we don't filter txn1 ackSet so redeliver this position bitSet is 111111
When filter the message we should filter the bit sit witch is real ack or in pendingAck state
add the test

(cherry picked from commit e0c0d5e)
(cherry picked from commit 90b4f86)
Technoboy- pushed a commit that referenced this pull request Mar 6, 2023
…t redevlier duplicate messages (#14327)

### Motivation
If individual ack batch message with transaction and abort this transaction, we will redeliver this message. but this batch message some bit sit are acked by another transaction and re consume this bit sit will produce `TransactionConflictException`, we don't need to redeliver this bit sit witch is acked by another transaction.

if batch have batch size 5

1. txn1 ack 0, 1     the ackSet is   00111
2. txn2 ack 2 3 4 the ack Set is  11000
3. abort txn2 redeliver this position is 00111
4. but now we don't filter txn1 ackSet so redeliver this position bitSet is 111111
### Modifications
When filter the message we should filter the bit sit witch is real ack or in pendingAck state
### Verifying this change
add the test
@michaeljmarshall
Copy link
Member

As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label

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.

9 participants