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++] add chunk message id #14604

Closed
wants to merge 26 commits into from

Conversation

A-Wanderer
Copy link

@A-Wanderer A-Wanderer commented Mar 8, 2022

Fixes #12402
Master Issue: #12402

Motivation

This is a feature which follow-up to the C++ Client,master issue is #12402.
Currently, when we send chunked messages, the producer returns the message-id of the last chunk. This can cause some problems. For example, when we use this message-id to seek, it will cause the consumer to consume from the position of the last chunk, and the consumer will mistakenly think that the previous chunks are lost and choose to skip the current message. If we use the inclusive seek, the consumer may skip the first message, which brings the wrong behavior.

API Changes and Implementation

  1. Add a new Message ID Impl type: Chunk Message ID Impl. The chunk message id Impl inherits from MessageIdImpl and adds two new methods: getFirstChunkMessageId and getLastChunkMessageID. For other method implementations, the lastChunkMessageID is called directly, which is compatible with much of the existing business logic.

In cosumer.seek, use the first chunk message-id param of the chunk message-id. This will solve the problem caused by seeking chunk messages.

  1. Add ProducerChunkedMessageCtx to stroing relevant chunk message id in Produceimpl,it mainly contains two public param: firstChunkMessageIdImplPtr_ and lastChunkMessageIdImplPtr_ to save information of chunk message id.

  2. Add some test about Seek and Chunk Message Id Impl.


Here is the PR to demonstrate this PIP: #12403.

@github-actions
Copy link

github-actions bot commented Mar 8, 2022

@A-Wanderer: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)

@BewareMyPower
Copy link
Contributor

Please complete the PR description. See more details in https://github.com/apache/pulsar/blob/master/CONTRIBUTING.md.

@BewareMyPower
Copy link
Contributor

In addition, please format your code via clang-format. You can run docker-format.sh simply or use your local clang-format (the version should be 5.0 because there are some incompatibilities with latest clang-format). See https://github.com/apache/pulsar/tree/master/pulsar-client-cpp#requirements-for-contributors.

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

I left some comments first, PTAL.

@A-Wanderer A-Wanderer force-pushed the ready_chunk_message_id branch from c15b225 to d119050 Compare March 8, 2022 17:29
@BewareMyPower BewareMyPower added component/c++ doc-not-needed Your PR changes do not impact docs labels Mar 9, 2022
@BewareMyPower BewareMyPower added this to the 2.11.0 milestone Mar 9, 2022
@github-actions
Copy link

github-actions bot commented Mar 9, 2022

@A-Wanderer:Thanks for providing doc info!

Co-authored-by: ran <gaoran_10@126.com>
@BewareMyPower BewareMyPower changed the title add chunk message id [C++] add chunk message id Mar 11, 2022
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

I've left some comments. Please address them first.

A-Wanderer and others added 10 commits March 14, 2022 13:59
Enhance the readability of judgment

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Delete redundant

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
const construction parameters

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
@BewareMyPower
Copy link
Contributor

I will check the compile failure on Windows x64 soon.

But your code doesn't pass the format check as well, please format the code via clang-format.

A-Wanderer and others added 4 commits March 22, 2022 00:01
@A-Wanderer A-Wanderer force-pushed the ready_chunk_message_id branch from 4cf8463 to d0b3a39 Compare March 22, 2022 19:40
const func

Co-authored-by: Yunze Xu <xyzinfernity@163.com>
@A-Wanderer A-Wanderer closed this Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PIP 107: Introduce the chunk message ID
3 participants