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

consumer: shut down on OffsetOutOfRange #424

Merged
merged 2 commits into from
Apr 16, 2015

Conversation

eapache
Copy link
Contributor

@eapache eapache commented Apr 16, 2015

The old behaviour was to redispatch it (so it would go to another broker if
necessary) and then retry with the same offset, which was a rather useless thing
to do unless the offset had somehow ended up slightly ahead of the available
messages (which is unlikely - it is far more likely to fall behind).

Instead, simply shut down the PartitionConsumer so the user gets an error (if
they're subscribed) and their messages channel closes. They then get to choose
whether to give up and switch to a different offset, yell for a human, or
whatever.

@Shopify/kafka make sense?

@eapache
Copy link
Contributor Author

eapache commented Apr 16, 2015

@wvanbergen I'm particularly curious what this behavioural change will do to the consumergroup code you're maintaining. I imagine you must already be detecting/handling this case from the Errors() channel and shutting down on it may cause problems?

@wvanbergen
Copy link
Contributor

In general 👍 from me.

  • How can the user detect that the consumer has stopped, or is going to stop, besides from knowing that this particular error is handled that way?
  • There's a theoretical case where this error will eventually go away: when the offset you're requesting is too high, but will eventually exist as new messages are produced to the partition. This can only happen during the initial Fetch request, and should be resolved by Better consumer offset handling #378
  • I think the godoc will need updating, because somewhere we say that the consumer never closes.

W.r.t. the consumergroup library: I don't think this is handled at all right now, so this will not make it any worse :)

@eapache
Copy link
Contributor Author

eapache commented Apr 16, 2015

How can the user detect that the consumer has stopped

Their message channel closes and they'll get an error if Return.Errors is true. I'm not sure what you mean?

I think the godoc will need updating, because somewhere we say that the consumer never closes.

Good catch, done.

@eapache eapache force-pushed the stop-consuming-offset-out-of-range branch from 912687c to 20275be Compare April 16, 2015 15:57
eapache added 2 commits April 16, 2015 16:48
The old behaviour was to redispatch it (so it would go to another broker if
necessary) and then retry with the same offset, which was a rather useless thing
to do unless the offset had somehow ended up slightly ahead of the available
messages (which is unlikely - it is far more likely to fall behind).

Instead, simply shut down the PartitionConsumer so the user gets an error (if
they're subscribed) and their messages channel closes. They then get to choose
whether to give up and switch to a different offset, yell for a human, or
whatever.
@eapache eapache force-pushed the stop-consuming-offset-out-of-range branch from 20275be to d82cd1f Compare April 16, 2015 16:48
eapache added a commit that referenced this pull request Apr 16, 2015
@eapache eapache merged commit ac59994 into master Apr 16, 2015
@eapache eapache deleted the stop-consuming-offset-out-of-range branch April 16, 2015 18: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