Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Transaction] Register transaction metadata before send or ack messages #8493

Merged
merged 4 commits into from
Nov 13, 2020

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Nov 9, 2020

Fixes https://github.com/streamnative/pulsar/issues/1659

Motivation

The transaction metadata registration should be the precondition of send or ack transaction messages, this guarantees TC could notify all related topic partitions and subscriptions successfully.

Modifications

  1. Register topic partition info to TC before sending transaction messages.
  2. Register subscription info to TC before ack transaction messages.

Verifying this change

This change is already covered by existing tests.

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)

@jiazhai
Copy link
Member

jiazhai commented Nov 10, 2020

/pulsarbot run-failure-checks

Comment on lines +479 to +480
.thenCompose(ignored -> doAcknowledge(messageIdList, ackType, properties, txn));
txn.registerAckOp(ackFuture);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we handle the exception here? If the metadata register failed, we can't send message or ack

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie
Copy link
Member

sijie commented Nov 12, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 812166b into apache:master Nov 13, 2020
@gaoran10 gaoran10 deleted the txn-register-before-send branch November 13, 2020 01:37
flowchartsman pushed a commit to flowchartsman/pulsar that referenced this pull request Nov 17, 2020
…es (apache#8493)

The transaction metadata registration should be the precondition of send or ack transaction messages, this guarantees TC could notify all related topic partitions and subscriptions successfully.
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.

[Transaction] Register transaction metadata before send or ack
4 participants