-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add documentation for handling rebalance #150
base: master
Are you sure you want to change the base?
Add documentation for handling rebalance #150
Conversation
README.md
Outdated
which requires you to return a `RebalanceCallback` structure which describes what actions should be performed in a certain situation. | ||
It allows you to use some of consumer's methods as well as a way to run an arbitrary computation. | ||
Please note that all the actions are executed on the consumer `poll` thread which means that running heavy or | ||
long-running computations is discouraged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
heavy or long-running computations is discouraged.
I think we need to add some short explanation why is it discouraged and what are the options if it's really really neded :)
- discouraged coz if it takes too long then kafka consumer instance can be removed from consumer group (default is 5 minutes and then consumer instance considered dead roughly speaking), but most probably before those 5 default minutes we would fail with timeout exception (default is 1 minute there) in
ToTry
- so to workaround those limitations user can adjust ToTry/max.poll.interval.ms timeouts
I realise now it's really hard to explain such important mechanics in fewer words, so mb it deservs a link to a dedicated section about ToTry/max.poll.interval.ms timeouts?
More details are available in following issues:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some clarifications - please check now, will that be enough?
README.md
Outdated
|
||
What you can currently do: | ||
- lift a pure value via `RebalanceCallback.pure(a)` | ||
- raise an error via `RebalanceCallback.fromTry(Failure(error))` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at #146 yet, but mb we would be able to raise error via ApplicativeError
after merging it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, didn't mean to create any confusion, sorry :)
I'm ok with any of the following options
- merge Error handling with MonadError for RebalanceCallback #146 before this PR, and then mention error handling in this PR
- merge Error handling with MonadError for RebalanceCallback #146 after this PR, and mention error handling in the docs within Error handling with MonadError for RebalanceCallback #146
- merge Error handling with MonadError for RebalanceCallback #146 after this and reflect error handling in the docs via new dedicated PR
feel free to pick the one which is more convenient for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a short description regarding raising errors as well as handling. Please check if that will be enough
Since #146 is merged, I will make a note about error handling in the docs |
No description provided.