Visitar URL original
[improve][broker] PIP-192: Add large topic count filter by Demogorgon314 · Pull Request #19613 · apache/pulsar · GitHub
Skip to content

[improve][broker] PIP-192: Add large topic count filter - #19613

Merged
Technoboy- merged 3 commits into
apache:masterfrom
Demogorgon314:Demogorgon314/PIP-192-Add_LargeTopicCountFilter
Mar 2, 2023
Merged

Technoboy- merged 3 commits into
apache:masterfrom
Demogorgon314:Demogorgon314/PIP-192-Add_LargeTopicCountFilter

Conversation

@Demogorgon314

Copy link
Copy Markdown
Member

Master Issue: #16691

Motivation

We will start raising PRs to implement PIP-192, #16691

Modifications

This PR added a filter that can filter brokers based on the number of topics.

TODO

The broker load data might be delayed, so the max topic check might not be accurate.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@Demogorgon314 Demogorgon314 self-assigned this Feb 23, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Feb 23, 2023
var brokerStats = pulsarStats.getBrokerStats();
var bundleStats = pulsarStats.getBundleStats();
var topics = 0;
for (Map.Entry<String, NamespaceBundleStats> bundleStatsEntry : bundleStats.entrySet()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define topics in BrokerStats and pre-aggregate this topic counts in updateStats?


   public synchronized void updateStats(
            ConcurrentOpenHashMap<String, ConcurrentOpenHashMap<String, ConcurrentOpenHashMap<String, Topic>>>
                    topicsMap) {
...
                    bundles.forEach((bundle, topics) -> {
                        NamespaceBundleStats currentBundleStats = bundleStats.computeIfAbsent(bundle,
                                k -> new NamespaceBundleStats());
                        currentBundleStats.reset();
                        currentBundleStats.topics = topics.size();
                        brokerStats.topics += topics.size(); //pre-aggregate

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, pre-aggregate will be better. Updated.

@gaoran10 gaoran10 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, maybe we can name the filter as MaxTopicCountFilter or BrokerMaxTopicCountFilter?

@Technoboy- Technoboy- added this to the 3.0.0 milestone Mar 2, 2023
@Technoboy- Technoboy- closed this Mar 2, 2023
@Technoboy- Technoboy- reopened this Mar 2, 2023
@Demogorgon314
Demogorgon314 requested a review from gaoran10 March 2, 2023 03:12
@Technoboy- Technoboy- added type/feature The PR added a new feature or issue requested a new feature area/broker labels Mar 2, 2023
@Technoboy-
Technoboy- merged commit fec4578 into apache:master Mar 2, 2023
@Demogorgon314
Demogorgon314 deleted the Demogorgon314/PIP-192-Add_LargeTopicCountFilter branch March 3, 2023 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs ready-to-test type/feature The PR added a new feature or issue requested a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants