-
Notifications
You must be signed in to change notification settings - Fork 141
Make sure producer can shut down #1590
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
base: master
Are you sure you want to change the base?
Conversation
I understand the problem, but it feels a bit weird to have to take into account everywhere in the zio-kafka code that it may be in an uninterruptible region and punch holes in it where needed. If we fix it here, there are probably 5 places we still miss. Feels like this should be solved at the Reloadable level in the zio core. Maybe you guys already explored this direction, I haven't read all the conversations on that topic. |
Uninterruptable ZIO's don't compose very well. Do you think there could be a general solution? On the other hand, fibers that are attached to a scope (.forkScoped) will always need to be interruptable. How else can they be stopped? |
Would it be hard to make a unit test for this? I wonder if there's alternative solutions, like wrapping the entire |
There seems to be a similar test in zio/zio#8837, which might serve as an inspiration.
That would also work, as discussed in #1590 (comment), but it unnecessarily widens the interruptible region and makes unrelated effects interruptible. Making only the fibers we expect to be interrupted interruptible would probably be more precise. On the other hand, if critical sections that must not be interrupted are already wrapped in |
Thanks for the suggestion. I looked at the test but it looks super tricky. I really can't follow how its supposed to test what the description says. I am afraid that we would need a maintainer with really deep ZIO skills to maintain something like this. (Or a PR with really well written comments 😉 ) |
That is also my point of view. Otherwise we might as well revert zio/zio#8638! BTW, I believe @svroonland is okay with this. He is only asking for some unit tests. Fair enough. Unfortunately, I do not know how to approach it. @Mee-Tree do you perhaps have a more concrete suggestion? |
As documented in #1588, when a producer is constructed in an uninterruptable region, it can not shut down. We fix this by running the critical parts in an interruptable region.
Fixes #1588.