Skip to content

[Transaction] Consume transaction message logic optimized #7833

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

Merged
merged 13 commits into from
Aug 31, 2020

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Aug 18, 2020

Motivation

The transaction message consuming logic needs to be optimized. One transaction commit marker in the topic partition consists of many Entries in the TransactionBuffer, so one transaction could be seen as one batch.

Modifications

  1. When sending messages to consumers use the batchIndex in the MessageIdData to present the startBatchIndex of the Entry.

  2. Calculate the transaction messages number.

  3. When the first reading one transaction init it's batchDeletedIndexes in the ManagedCursorImpl.

Verifying this change

This change can be verified as follows:

  • org.apache.pulsar.broker.transaction.TransactionConsumeTest

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)

@gaoran10 gaoran10 changed the title [Transaction] Consume transaction message logic optimized optimized [Transaction] Consume transaction message logic optimized Aug 18, 2020
@codelipenghui codelipenghui added this to the 2.7.0 milestone Aug 18, 2020
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10 gaoran10 force-pushed the txn-consume-message-batch-index branch 2 times, most recently from 010d775 to ea9bbaf Compare August 21, 2020 02:04
@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

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Looks good, could you please add a test for the batch indexes of a transaction at the consumer side?

Comment on lines 2862 to 2866
if (!batchDeletedIndexes.containsKey(position)) {
BitSetRecyclable bitSetRecyclable = BitSetRecyclable.create();
bitSetRecyclable.set(0, totalNumMessageInBatch);
batchDeletedIndexes.put(position, bitSetRecyclable);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

batchDeletedIndexes.computeIfAbsent().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use this method instead.


- name: run unit tests for pulsar-broker
if: steps.docs.outputs.changed_only == 'no'
run: ./build/retry.sh mvn -B -ntp test -pl pulsar-broker -Dinclude='org/apache/pulsar/broker/**/*.java' -Dexclude='org/apache/pulsar/broker/zookeeper/**/*.java,org/apache/pulsar/broker/loadbalance/**/*.java,org/apache/pulsar/broker/service/**/*.java,**/AdminApiOffloadTest.java,**/TransactionProduceTest.java'
run: ./build/retry.sh mvn -B -ntp test -pl pulsar-broker -Dinclude='org/apache/pulsar/broker/**/*.java' -Dexclude='org/apache/pulsar/broker/zookeeper/**/*.java,org/apache/pulsar/broker/loadbalance/**/*.java,org/apache/pulsar/broker/service/**/*.java,**/AdminApiOffloadTest.java,org/apache/pulsar/broker/transaction/**/*.java,org/apache/pulsar/client/transaction/**/*.java'
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we need this change?

@gaoran10 gaoran10 force-pushed the txn-consume-message-batch-index branch from b19df42 to fde7cd4 Compare August 27, 2020 13:46
@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit b4b77be into apache:master Aug 31, 2020
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

The transaction message consuming logic needs to be optimized. One transaction commit marker in the topic partition consists of many Entries in the TransactionBuffer, so one transaction could be seen as one batch.

### Modifications

1. When sending messages to consumers use the `batchIndex` in the MessageIdData to present the startBatchIndex of the Entry.

2. Calculate the transaction messages number.

3. When the first reading one transaction init it's batchDeletedIndexes in the ManagedCursorImpl.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

The transaction message consuming logic needs to be optimized. One transaction commit marker in the topic partition consists of many Entries in the TransactionBuffer, so one transaction could be seen as one batch.

### Modifications

1. When sending messages to consumers use the `batchIndex` in the MessageIdData to present the startBatchIndex of the Entry.

2. Calculate the transaction messages number.

3. When the first reading one transaction init it's batchDeletedIndexes in the ManagedCursorImpl.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

The transaction message consuming logic needs to be optimized. One transaction commit marker in the topic partition consists of many Entries in the TransactionBuffer, so one transaction could be seen as one batch.

### Modifications

1. When sending messages to consumers use the `batchIndex` in the MessageIdData to present the startBatchIndex of the Entry.

2. Calculate the transaction messages number.

3. When the first reading one transaction init it's batchDeletedIndexes in the ManagedCursorImpl.
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

2 participants