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.

Sign up for GitHub

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

Jump to bottom

[Issue 172] Add key based batcher #400

Merged
merged 6 commits into from
Dec 1, 2020
Merged

[Issue 172] Add key based batcher #400

merged 6 commits into from
Dec 1, 2020

Conversation

freeznet
Copy link
Contributor

Fixes #172

Motivation

Add a new batch message container named keyBasedBatchContainer to support batching message in key_shared subscription mode.

Modifications

  • add BatchBuilder interface, add FlushBatches and IsMultiBatches func
  • change old BatchBuilder struct to batchContainer
  • add keyBasedBatchContainer
  • add tests

Verifying this change

This change added tests and can be verified as follows:

  • Added integration tests for key based batch producer with multiple consumer in KeyShared mode
  • Added integration tests for message ordering with key based batch producer and KeyShared consumer

@wolfstudy wolfstudy mentioned this pull request Nov 25, 2020
Closed
@freeznet
Copy link
Contributor Author

rerun failure checks

wolfstudy
wolfstudy reviewed Nov 30, 2020
View reviewed changes
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

Thanks @freeznet , great changes, just a little comment, please check.

pulsar/consumer_test.go Outdated
receivedMessageIndex++
}

// TODO: add OrderingKey support
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @freeznet work for this, can you help to create an issue to track this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new issue added #401, also add to comment to track.

pulsar/consumer_test.go Show resolved Hide resolved
@wolfstudy wolfstudy added this to the 0.4.0 milestone Nov 30, 2020
@freeznet freeznet mentioned this pull request Nov 30, 2020
Closed
@sijie sijie mentioned this pull request Nov 30, 2020
Closed
wolfstudy
wolfstudy approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

LGTM +1

@wolfstudy
Copy link
Member

@freeznet Please merge master code to fix conflict, thanks?

@freeznet
Copy link
Contributor Author

freeznet commented Dec 1, 2020

@freeznet Please merge master code to fix conflict, thanks?

resolved ;)

@wolfstudy wolfstudy merged commit 154bff0 into apache:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wolfstudy wolfstudy wolfstudy approved these changes

Assignees

@freeznet freeznet

Labels
area/producer
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.

Support key based batcher for key_shared
2 participants
@freeznet @wolfstudy