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] Produce transaction messages and commit #7552

Merged
merged 30 commits into from
Aug 10, 2020

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Jul 16, 2020

Master Issue: #2664

Motivation

Currently, the transaction components are all independent, the relationship between transaction client and transaction server needs to be established.

The target of this PR is making the Pulsar client could send transaction messages to the Pulsar broker and execute commit command.

Modifications

  1. Pulsar client generates a new transaction and gets the txnID.
  2. Pulsar client sends transaction messages to TransactionBuffer.
  3. The Transaction sends a commit command to TransactionBuffer.
  4. Append a committing marker in related TransactionBuffers.
  5. Append a committed marker in related topic partitions.
  6. Append a committed marker int related TransactionBuffers.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests

  • org.apache.pulsar.broker.transaction.PulsarClientTransactionTest.produceCommitTest

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: (yes)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (yes)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (don't know)

Documentation

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

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

4 similar comments
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10 gaoran10 changed the title [WIP] Produce transaction messages integration [Transaction] Produce transaction messages and commit Jul 20, 2020
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

2 similar comments
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10 gaoran10 force-pushed the tnx-client-server-tandem branch 2 times, most recently from 32081ba to 8fbcf12 Compare July 27, 2020 02:42
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

Comment on lines 556 to 557
transactionBufferProvider = TransactionBufferProvider.newProvider(
PersistentTransactionBufferProvider.class.getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to config in the broker.conf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll fix it.

@@ -1212,6 +1221,33 @@ protected void handleSend(CommandSend send, ByteBuf headersAndPayload) {

startSendOperation(producer, headersAndPayload.readableBytes(), send.getNumMessages());

final long produceId = producer.getProducerId();
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to handle the transaction message in the topic, not the server cnx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's more reasonable.

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

Comment on lines 1712 to 1696
.thenRun(() -> {
if (log.isDebugEnabled()) {
log.debug("Send response success for end txn request {}", command.getRequestId());
}
updateTBStatus(txnID, newStatus).thenRun(() -> {
service.pulsar().getTransactionMetadataStoreService()
.updateTxnStatus(txnID, TxnStatus.COMMITTED, TxnStatus.COMMITTING);
ctx.writeAndFlush(Commands.newEndTxnResponse(requestId,
txnID.getLeastSigBits(), txnID.getMostSigBits()));
}).exceptionally(throwable -> {
log.error("Failed to get txn `" + txnID + "` txnMeta.", throwable);
ctx.writeAndFlush(Commands.newEndTxnResponse(requestId, txnID.getMostSigBits(),
BrokerServiceException.getClientErrorCode(throwable), throwable.getMessage()));
return null;
});
}).exceptionally(throwable -> {
if (log.isDebugEnabled()) {
log.debug("Send response error for end txn request {}", command.getRequestId());
}
ctx.writeAndFlush(Commands.newEndTxnResponse(command.getRequestId(), txnID.getMostSigBits(),
BrokerServiceException.getClientErrorCode(throwable), throwable.getMessage()));
return null;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

We should handle it in the TC and TC should retry to make sure the TB can handle success.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll fix this.

@gaoran10
Copy link
Contributor Author

gaoran10 commented Aug 8, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit ba5c5c7 into apache:master Aug 10, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation

Currently, the transaction components are all independent, the relationship between transaction client and transaction server needs to be established.

The target of this PR is making the Pulsar client could send transaction messages to the Pulsar broker and execute commit command.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

Currently, the transaction components are all independent, the relationship between transaction client and transaction server needs to be established.

The target of this PR is making the Pulsar client could send transaction messages to the Pulsar broker and execute commit command.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

Currently, the transaction components are all independent, the relationship between transaction client and transaction server needs to be established.

The target of this PR is making the Pulsar client could send transaction messages to the Pulsar broker and execute commit command.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

Currently, the transaction components are all independent, the relationship between transaction client and transaction server needs to be established.

The target of this PR is making the Pulsar client could send transaction messages to the Pulsar broker and execute commit command.
@gaoran10 gaoran10 mentioned this pull request Nov 21, 2020
32 tasks
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.

None yet

3 participants