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

Verify checkpoint saving strategy #72

Closed
fbeltrao opened this issue Apr 11, 2019 · 5 comments · Fixed by #73
Closed

Verify checkpoint saving strategy #72

fbeltrao opened this issue Apr 11, 2019 · 5 comments · Fixed by #73
Assignees
Labels
question Further information is requested
Milestone

Comments

@fbeltrao
Copy link
Contributor

Checkpoint saving current is done using Consumer.Commit which blocks the thread. An alternative is to use StoreOffset that will save the checkpoint asynchronously in librdkafka.

Commit is more accurate while StoreOffset offers a better throughput.

Would love your feedback @jeffhollan, @anirudhgarg and @ryancrawcour

@fbeltrao fbeltrao added the question Further information is requested label Apr 11, 2019
@fbeltrao fbeltrao added this to the P1 milestone Apr 11, 2019
@jeffhollan
Copy link

I imagine the risk of StoreOffset is that if something happens on the async thread and the commit doesn't checkpoint you may end up in a world where you replay a few batches of messages you thought were being checkpoint but the checkpoint was never active? In general we are pretty clear you need to be at-least-once expected, so I think I'm ok with the StoreOffset given the better throughput.

The other alternative is we make it a configurable choice in the host.json config so they can choose the method they want and we default to one.

@ryancrawcour
Copy link
Contributor

ou need to be at-least-once expected, so I think I'm ok with the StoreOffset given the better throughput.

that's my feeling too

@ryancrawcour
Copy link
Contributor

we make it a configurable choice

that's not a bad idea. how complex would this make the code @fbeltrao ?

@fbeltrao
Copy link
Contributor Author

fbeltrao commented Apr 12, 2019

The code to support both is not complex. I am questioning the value.

What Jeff says is right, consumers should be implemented to process messages at least once anyway.

I can run a few tests to see if the messages processing repetition is noticeable in our e2e tests if we optimize for throughput.

@ryancrawcour
Copy link
Contributor

ryancrawcour commented Apr 12, 2019

I'd say let's pick one. Then if customers ask for the other one, or complain about the one we picked, we can then always come back and revisit this.

I am happy with "you're expected to ensure your consumer is idempotent b/c at-least-once semantics" and go for the higher throughput.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants