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

Simple batch produce #896

Merged
merged 8 commits into from
Mar 30, 2021
Merged

Simple batch produce #896

merged 8 commits into from
Mar 30, 2021

Conversation

nicholastmosher
Copy link
Contributor

@nicholastmosher nicholastmosher commented Mar 25, 2021

Subtask of #847

Here is the first shot at implementing the batch producer. I have included an example and everything is working. The only thing that is still up in the air is trying to see whether we can create a more flexible API.

Currently, send_all can take any argument that can turn into an iterator over (Option<Key>, Value) pairs. This is one example of how calling it looks:

let batch: Vec<_> = (0..10)
    .map(|i| (Some(i.to_string()), format!("This is record {}", i)))
    .collect();
producer.send_all(batch).await?;

@nicholastmosher
Copy link
Contributor Author

After a good bit of experimentation, I have not found a way to support all three calling patterns. The one that I have included in this PR is the most general one, and it is still pretty easy to use. @sehz I would like to move forward with merging this as-is, and if we want to prioritize new calling patterns in the future we can make a new issue for it.

@nicholastmosher nicholastmosher mentioned this pull request Mar 25, 2021
4 tasks
@nicholastmosher nicholastmosher linked an issue Mar 25, 2021 that may be closed by this pull request
4 tasks
src/client/src/producer.rs Show resolved Hide resolved
src/client/src/producer.rs Show resolved Hide resolved
src/dataplane-protocol/src/record.rs Show resolved Hide resolved
src/client/src/producer.rs Outdated Show resolved Hide resolved
@sehz
Copy link
Contributor

sehz commented Mar 29, 2021

rebase as well

src/client/src/producer.rs Outdated Show resolved Hide resolved
src/client/src/producer.rs Outdated Show resolved Hide resolved
src/client/src/producer.rs Outdated Show resolved Hide resolved
@nicholastmosher nicholastmosher removed a link to an issue Mar 30, 2021
4 tasks
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

LGTM

@sehz sehz merged commit 52fe50e into infinyon:master Mar 30, 2021
@nicholastmosher nicholastmosher deleted the batch-produce branch March 30, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants