Skip to content
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

Fetch the query quota capacity utilization rate broker metric in a callback function #12767

Merged
merged 1 commit into from
May 9, 2024

Conversation

jackjlli
Copy link
Member

@jackjlli jackjlli commented Apr 1, 2024

This PR fetches the query quota capacity utilization rate broker metric in a callback function.
Related issue: #12768

The reason is that this broker metric is a gauge metric. If there is no incoming query, the gauge value would remain the same without any updates. In addition, if the total query quota is less than the number of brokers, meaning if the per-broker rate is less than 1, the metric value would always be > 100. So even there is only 1 incoming query for a while, the metric value would still be > 100.

@Jackie-Jiang
Copy link
Contributor

@zhtaoxiang Can you help take a look?

Copy link
Contributor

@jasperjiaguo jasperjiaguo left a comment

Choose a reason for hiding this comment

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

I generally agree with the polling approach and the use of (perBrokerRate * finalQueryQuotaEntity.getMaxQpsTracker().getDefaultTimeRangeMs() / 1000L); looks correct to me. Minor comments.


@VisibleForTesting
@Override
int getHitCount(long now) {
Copy link
Contributor

@jasperjiaguo jasperjiaguo May 9, 2024

Choose a reason for hiding this comment

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

Should this be getHitCount()? Otherwise finalQueryQuotaEntity.getMaxQpsTracker().getHitCount(); would be calling into the base class impl? Am I missing smth here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! The call of finalQueryQuotaEntity.getMaxQpsTracker().getHitCount() will call the getHitCount() method from the base class, which internally invokes the int getHitCount(long now) method in the child class. So we should be good here.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh I see make sense

Copy link
Contributor

@jasperjiaguo jasperjiaguo left a comment

Choose a reason for hiding this comment

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

lgtm

@jackjlli jackjlli merged commit 984b561 into master May 9, 2024
19 checks passed
@jackjlli jackjlli deleted the fix-broker-metrics branch May 9, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants