KAFKA-16954: fix consumer close to release assignment in background#16343
KAFKA-16954: fix consumer close to release assignment in background#16343lucasbru merged 3 commits intoapache:trunkfrom
Conversation
|
@jlprat we would need this blocker into 3.8. Note this is not production-ready code since KIP-848 in only going out as Preview in 3.8 (so pushing this fix for a race condition affecting consumer close that we discovered, to make KIP-848 preview better). Makes sense? Thanks! |
|
Is there any way to have this merged before we start making RCs for 3.8.0? |
|
@lianetm The general idea looks good to me, but we have a lot of failing integration tests with this change. |
|
Hey @lucasbru , solved. Issue was that all those integration tests do close with zero timeout, and in that scenario the legacy logic does not propagate a timeout exception while leaving the group (and I was propagating it here). The legacy logic just triggers the leave and waits if it has time. If it doesn't, it just logs a warn here). So I just did the same on the new consumer. Tried several of the integration test files that were failing and they pass locally now. I'm running them all now while the build here completes. Thanks! |
|
Hey @lucasbru , build completed with 6 unrelated test failures:
Thanks! |
…pache#16343) This PR fixes consumer close to avoid updating the subscription state object in the app thread. Now the close simply triggers an UnsubscribeEvent that is handled in the background to trigger callbacks, clear assignment, and send leave heartbeat. Note that after triggering the event, the unsubscribe will continuously process background events until the event completes, to ensure that it allows for callbacks to run in the app thread. The logic around what happens if the unsubscribe fails remain unchanged: close will log, keep the first exception and carry on. It also removes the redundant LeaveOnClose event (it used to do the the exact same thing as the UnsubscribeEvent, both calling membershipMgr.leaveGroup). Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
…pache#16343) This PR fixes consumer close to avoid updating the subscription state object in the app thread. Now the close simply triggers an UnsubscribeEvent that is handled in the background to trigger callbacks, clear assignment, and send leave heartbeat. Note that after triggering the event, the unsubscribe will continuously process background events until the event completes, to ensure that it allows for callbacks to run in the app thread. The logic around what happens if the unsubscribe fails remain unchanged: close will log, keep the first exception and carry on. It also removes the redundant LeaveOnClose event (it used to do the the exact same thing as the UnsubscribeEvent, both calling membershipMgr.leaveGroup). Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
…16376) * MINOR: Improving log for outstanding requests on close and cleanup (#16304) Reviewers: Andrew Schofield <aschofield@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com> * KAFKA-16954: fix consumer close to release assignment in background (#16343) This PR fixes consumer close to avoid updating the subscription state object in the app thread. Now the close simply triggers an UnsubscribeEvent that is handled in the background to trigger callbacks, clear assignment, and send leave heartbeat. Note that after triggering the event, the unsubscribe will continuously process background events until the event completes, to ensure that it allows for callbacks to run in the app thread. The logic around what happens if the unsubscribe fails remain unchanged: close will log, keep the first exception and carry on. It also removes the redundant LeaveOnClose event (it used to do the the exact same thing as the UnsubscribeEvent, both calling membershipMgr.leaveGroup). Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
…pache#16343) This PR fixes consumer close to avoid updating the subscription state object in the app thread. Now the close simply triggers an UnsubscribeEvent that is handled in the background to trigger callbacks, clear assignment, and send leave heartbeat. Note that after triggering the event, the unsubscribe will continuously process background events until the event completes, to ensure that it allows for callbacks to run in the app thread. The logic around what happens if the unsubscribe fails remain unchanged: close will log, keep the first exception and carry on. It also removes the redundant LeaveOnClose event (it used to do the the exact same thing as the UnsubscribeEvent, both calling membershipMgr.leaveGroup). Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
3 participants
This PR fixes consumer close to avoid updating the subscription state object in the app thread. Now the close simply triggers an UnsubscribeEvent that is handled in the background to trigger callbacks, clear assignment, and send leave heartbeat. Note that after triggering the event, the unsubscribe will continuously process background events until the event completes, to ensure that it allows for callbacks to run in the app thread.
The logic around what happens if the unsubscribe fails remain unchanged: close will log, keep the first exception and carry on.
It also removes the redundant LeaveOnClose event (it used to do the the exact same thing as the UnsubscribeEvent, both calling membershipMgr.leaveGroup).