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

[C++] Fix message id error if messages were sent to a partitioned topic #6938

Merged
merged 1 commit into from May 12, 2020

Conversation

BewareMyPower
Copy link
Contributor

Motivation

If messages were sent to a partitioned topic, the message id's partition field was always -1 because SendReceipt command only contains ledger id and entry id.

Modifications

  • Add a partition field to ProducerImpl and set the MessageId's partition field with it in ackReceived method later.
  • Add a test to check message id in send callback if messages were sent to a partitioned topic.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as BasicEndToEndTest.testSendCallback.

@BewareMyPower BewareMyPower changed the title Fix message id error if messages were sent to a partitioned topic [C++] Fix message id error if messages were sent to a partitioned topic May 11, 2020
@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented May 12, 2020

Thanks @BewareMyPower for the fix.

@jiazhai jiazhai added component/c++ release/2.5.2 type/bug The PR fixed a bug or issue reported a bug labels May 12, 2020
@jiazhai jiazhai added this to the 2.6.0 milestone May 12, 2020
@jiazhai jiazhai merged commit 15cb920 into apache:master May 12, 2020
@jiazhai jiazhai mentioned this pull request May 12, 2020
1 task
jiazhai pushed a commit that referenced this pull request May 12, 2020
)

### Motivation

If messages were sent to a partitioned topic, the message id's `partition` field was always -1 because SendReceipt command only contains ledger id and entry id.

### Modifications

- Add a `partition` field to `ProducerImpl` and set the `MessageId`'s `partition` field with it in `ackReceived` method later.
- Add a test to check message id in send callback if messages were sent to a partitioned topic.

(cherry picked from commit 15cb920)
Huanli-Meng pushed a commit to Huanli-Meng/pulsar that referenced this pull request May 27, 2020
…ache#6938)

### Motivation

If messages were sent to a partitioned topic, the message id's `partition` field was always -1 because SendReceipt command only contains ledger id and entry id.

### Modifications

- Add a `partition` field to `ProducerImpl` and set the `MessageId`'s `partition` field with it in `ackReceived` method later.
- Add a test to check message id in send callback if messages were sent to a partitioned topic.
addisonj pushed a commit to instructure/pulsar that referenced this pull request Jun 12, 2020
…ache#6938)

### Motivation

If messages were sent to a partitioned topic, the message id's `partition` field was always -1 because SendReceipt command only contains ledger id and entry id.

### Modifications

- Add a `partition` field to `ProducerImpl` and set the `MessageId`'s `partition` field with it in `ackReceived` method later.
- Add a test to check message id in send callback if messages were sent to a partitioned topic.

(cherry picked from commit 15cb920)
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
…ache#6938)

### Motivation

If messages were sent to a partitioned topic, the message id's `partition` field was always -1 because SendReceipt command only contains ledger id and entry id.

### Modifications

- Add a `partition` field to `ProducerImpl` and set the `MessageId`'s `partition` field with it in `ackReceived` method later.
- Add a test to check message id in send callback if messages were sent to a partitioned topic.
@BewareMyPower BewareMyPower deleted the sendcb-dev branch September 16, 2022 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/2.5.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants