fix: delete zombie consumer groups 🧟 #6160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6163242 to
55d72a7
Compare
vpapavas
reviewed
Sep 10, 2020
ksqldb-engine/src/main/java/io/confluent/ksql/engine/QueryCleanupService.java
Outdated
Show resolved
Hide resolved
vpapavas
reviewed
Sep 10, 2020
|
|
||
| @Override | ||
| public void run() { | ||
| tryRun( |
There was a problem hiding this comment.
Do we want these three cleanups to happen atomically, i.e all or nothing?
There was a problem hiding this comment.
no, if one of them fails we should still try to clean the others
vpapavas
reviewed
Sep 10, 2020
| () -> adminClient.get().deleteConsumerGroups(groups).all().get(), | ||
| e -> (e instanceof RetriableException) | ||
| || (e instanceof GroupNotEmptyException && retryCount.getAndIncrement() < 5), | ||
| () -> Duration.of(3, ChronoUnit.SECONDS) |
There was a problem hiding this comment.
Why not use the actual value of heartbeat.interval.ms? Why hardcode it to 3 seconds?
There was a problem hiding this comment.
it's a consumer config so it can be different for each consumer (and unfortunately this isn't a constant in the kafka code, so I can't just use a constant from that class directly)
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1283
Description
Before this PR, we don't clean up consumer groups that are leftover which makes it really annoying for users to look into their existing consumer groups. This PR adds cleanup on close for persistent and transient queries.
The unfortunate part, is that the consumer group API takes a while to update when the last consumer has left the group (specifically, the
heartbeat.interval.mswhich is default to 3 seconds, needs to pass after the last consumer leaves the group). To account for this, I've taken the external resource cleanup out of the main execution path so that the UI doesn't hang when we terminate a query.Testing done
Unit testing, and manual testing to make sure the resources are cleaned up.
Reviewer checklist