Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Change offset commit behaviour to be in line with Java consumers #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ducas
Copy link
Contributor

@ducas ducas commented Apr 3, 2016

… versus previously consumed offset. Fixes #27

@mhorstma
Copy link
Contributor

mhorstma commented Apr 6, 2016

What would be the risk of making this the default behavior, since it seems that this is a more correct way of managing committed offsets? The expansion of various little config settings is starting to get a little crazy.

@ducas
Copy link
Contributor Author

ducas commented Apr 6, 2016

i agree that a config setting for everything can get crazy. i would like to
see parity with the java consumer configuration though...

i can run a few tests to see if you get duplicate messages or not. the only
other problem i can think of is that it will affect monitoring tools -
which is the reason i would like to fix it... :-)

On Thu, Apr 7, 2016 at 3:10 AM mhorstma [email protected] wrote:

What would be the risk of making this the default behavior, since it seems
that this is a more correct way of managing committed offsets? The
expansion of various little config settings is starting to get a little
crazy.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#45 (comment)

Cheers,
Ducas

@mhorstma
Copy link
Contributor

mhorstma commented Apr 6, 2016

Your point of keeping parity with the Java consumer config is a good one. I think this change looks good.

@ducas ducas changed the title Add configurable behavior for committing the next fetch Change offset commit behaviour to be in line with Java consumers Apr 7, 2016
@ducas
Copy link
Contributor Author

ducas commented Apr 7, 2016

@mhorstma how does this look now...?

@mhorstma
Copy link
Contributor

mhorstma commented Apr 7, 2016

Looks great to me. Let's give others a little time to weigh in, if they wish, before merging.

this brings the behavior of the balanced consumer in line with the java implementation
@ducas ducas force-pushed the fix/offset-off-by-one branch from b87b743 to 9fc7b5c Compare April 28, 2016 06:26
@ducas
Copy link
Contributor Author

ducas commented Apr 28, 2016

i've rejigged and rebased this PR as #46 caused conflicts

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants