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

new feature #73

Closed
vtolstov opened this issue Aug 27, 2021 · 6 comments
Closed

new feature #73

vtolstov opened this issue Aug 27, 2021 · 6 comments

Comments

@vtolstov
Copy link
Contributor

what you think about semi-autocommit feature?
you already have CommitRecords func, but if i want to specify autocommit interval and CommitRecords not commit passed records immediately but in provided interval. So i can call CommitRecords with 1,2,3,5 records that not commited right now, but accumulated. For example i'm specify autocommit interval 5s, in this case i can call CommitRecords multiple times with one or many records and after interval happens - client send commit offsets request to kafka with accumulated offsets.
What you think ?

@twmb twmb mentioned this issue Aug 31, 2021
@twmb
Copy link
Owner

twmb commented Aug 31, 2021

I think this is a good idea, but I don't want to merge the behavior with CommitRecords, because merging it would mean no synchronous usage CommitRecords.

I think this can be done with a new option and a new client method:

func AutoCommitMarks() GroupOpt

func (cl *Client) MarkCommitRecords(...*Record)

I'm not yet 100% settled on the names, but I'm leaning heavily towards what's proposed above. I'll commit this probably tomorrow, as a part of v0.11.0, which will come with a few API breakages (detailed in #62).

@vtolstov
Copy link
Contributor Author

nice, will look at it after merging

@twmb
Copy link
Owner

twmb commented Aug 31, 2021

Is the goal here to also opt out of the current behavior that allows any prior polled records to be committed during the next poll?

Right now if you poll offsets 0 through 5, the next poll will allow 5 to be committed. I can't decide whether to opt out of this such that you must MarkCommitRecords, or whether to keep this behavior so that you only need to use MarkCommitRecords if you want to have records available for commit while processing a batch slowly.

@twmb
Copy link
Owner

twmb commented Aug 31, 2021

I'm leaning towards making MarkCommitRecords required, I think that feels more understandable and gives more control.

@twmb twmb closed this as completed in 31ed46b Sep 1, 2021
@twmb
Copy link
Owner

twmb commented Sep 1, 2021

This has been implemented in 31ed46b

@twmb
Copy link
Owner

twmb commented Sep 1, 2021

And tagged in v0.10.3

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

No branches or pull requests

2 participants