-
Notifications
You must be signed in to change notification settings - Fork 3.7k
On multi-topic consumer, we shouldn't keep checking the partitioned metadata #10708
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
On multi-topic consumer, we shouldn't keep checking the partitioned metadata #10708
Conversation
eolivelli
left a comment
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.
Lgtm
I left a minor comment
| .collect(Collectors.toList()); | ||
| } else { | ||
| boolean isTopicBeingSubscribedForInOtherThread = this.topics.putIfAbsent(topicName, 1) != null; | ||
| boolean isTopicBeingSubscribedForInOtherThread = this.topics.putIfAbsent(topicName, 0) != null; |
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.
Does it make sense to create a constant for this magic value?
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.
Good point. Added constant
…etadata (apache#10708) * On multi-topic consumer, we shouldn't keep checking the partitioned metadata * Added NON_PARTITIONED constant * Removed assertion that is now invalid * Fixed handling of deleted partitioned topic * Fixed re-subscribing same topic
…etadata (apache#10708) * On multi-topic consumer, we shouldn't keep checking the partitioned metadata * Added NON_PARTITIONED constant * Removed assertion that is now invalid * Fixed handling of deleted partitioned topic * Fixed re-subscribing same topic
… partitioned metadata (apache#10708)
…partitioned metadata (#11168) Pick #10708 to branch 2.7. ### Motivation When using a multi-topic consumer, either through the regex or by passing a list of topics, we are ending up checking the partitioned topic metadata every 1 min, even if the topics are not partitioned. We don't need to re-check that information if the topics are not partitioned. Also, this can be causing a lot of read traffic on ZK since we keep checking that "partitioned-metadata" that these topics don't have and, in 2.7, this information is not cached in brokers when the metadata is not found. ### Modifications When adding non-partitioned topics to the multi-topic consumer, use 0 for partitions when the topic is not partitioned in order to skip the periodic re-check of the partitions count.
…etadata (apache#10708) * On multi-topic consumer, we shouldn't keep checking the partitioned metadata * Added NON_PARTITIONED constant * Removed assertion that is now invalid * Fixed handling of deleted partitioned topic * Fixed re-subscribing same topic
…etadata (apache#10708) * On multi-topic consumer, we shouldn't keep checking the partitioned metadata * Added NON_PARTITIONED constant * Removed assertion that is now invalid * Fixed handling of deleted partitioned topic * Fixed re-subscribing same topic
Motivation
When using a multi-topic consumer, either through the regex or by passing a list of topics, we are ending up checking the partitioned topic metadata every 1 min, even if the topics are not partitioned.
We don't need to re-check that information if the topics are not partitioned. Also, this can be causing a lot of read traffic on ZK since we keep checking that "partitioned-metadata" that these topics don't have and, in 2.7, this information is not cached in brokers when the metadata is not found.
Modifications
When adding non-partitioned topics to the multi-topic consumer, use 0 for partitions when the topic is not partitioned in order to skip the periodic re-check of the partitions count.