Repository navigation
Kafka support manual partition pause/resume with pause-if-no-requests=true - #3538
ozangunalp wants to merge 1 commit into
Conversation
7c4e746 to
96a4098
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3538 +/- ##
============================================
+ Coverage 77.47% 77.93% +0.46%
- Complexity 3778 5641 +1863
============================================
Files 306 487 +181
Lines 12673 18842 +6169
Branches 1648 2319 +671
============================================
+ Hits 9818 14685 +4867
- Misses 2116 3009 +893
- Partials 739 1148 +409
🚀 New features to boost your workflow:
|
96a4098 to
4695f0e
Compare
| } | ||
|
|
||
| void removeManuallyPausedPartitions(Collection<TopicPartition> partitions) { | ||
| if (manuallyPausedPartitions.removeAll(partitions)) { |
There was a problem hiding this comment.
Is this correct? removeAll returns true if it removes at least one partition from the collection. I feel like you want to remove all of them. If partitions {tp0, tp1, tp2} are revoked but only tp0 was manually paused, the log says "Cleaning up paused partitions [tp0, tp1, tp2] due to revocation". Slightly misleading, no?.
There was a problem hiding this comment.
Nice catch, we should log the intersection if there are any.
There was a problem hiding this comment.
Instead I added both revoked and paused partitions to the log.
There was a problem hiding this comment.
I don't believe it's correct.
removeAll mutates manuallyPausedPartitions before the log call. The log message template is: "Cleaning up manually paused partitions %s due to revocation of %s".
The first %s receives the remaining set, not the cleaned-up set.
I think you need:
void removeManuallyPausedPartitions(Collection<TopicPartition> partitions) {
Set<TopicPartition> removed = new HashSet<>(manuallyPausedPartitions);
removed.retainAll(partitions);
if (!removed.isEmpty()) {
manuallyPausedPartitions.removeAll(removed);
log.cleaningUpPausedPartitions(removed, partitions);
}
}
There was a problem hiding this comment.
or update the log messsage: "Remaining manually paused partitions %s after revocation of %s"
There was a problem hiding this comment.
Yes, let's change the log message.
4695f0e to
ace0a93
Compare
ace0a93 to
ee98dc5
Compare
ee98dc5 to
3f3a63f
Compare
Fixes #3295