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

adds ResetOffset to reset to earlier offset values. #932

Merged
merged 1 commit into from
Sep 12, 2017

Conversation

fgeller
Copy link
Contributor

@fgeller fgeller commented Aug 18, 2017

This is mostly meant as a start of a discussion, happy to change it. cf #554 for initial discussions and use cases.

Added ResetOffset on PartitionOffsetManager that allows setting the offset to a value less than the current value.

From a downstream perspective I would prefer to have a single func to call that allows setting an offset to a smaller or greater value, regardless of the current value - but thought API-wise it'd make more sense to have MarkOffset and ResetOffset act as duals/complements. What do you think?

@eapache
Copy link
Contributor

eapache commented Aug 21, 2017

I'm quite happy with how this looks, thanks!

@wvanbergen you had some thoughts on this originally.

@eapache eapache merged commit be3a4e3 into IBM:master Sep 12, 2017
pom.lock.Lock()
defer pom.lock.Unlock()

if offset < pom.offset {
Copy link

Choose a reason for hiding this comment

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

@eapache @fgeller sorry I'm late to the party on this, just now getting around to implement our usage of this feature.

Any reason for this condition? this is preventing me from updating the metadata without changing the offset.

Copy link
Contributor

Choose a reason for hiding this comment

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

updating the metadata without changing the offset.

I don't think either of us ever considered a use case for that. I suppose it would be easy enough to make this a <=

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

3 participants