Skip to content

Make the server query meter table level#18882

Merged
Jackie-Jiang merged 2 commits into
apache:masterfrom
J-HowHuang:table-level-server-query-meter
Jul 1, 2026
Merged

Make the server query meter table level#18882
Jackie-Jiang merged 2 commits into
apache:masterfrom
J-HowHuang:table-level-server-query-meter

Conversation

@J-HowHuang

@J-HowHuang J-HowHuang commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Description

The query meter didn't carry table information since #9001. Adding it so we can track table level QPS

@J-HowHuang

Copy link
Copy Markdown
Collaborator Author

@Jackie-Jiang is it better to add a new meter with a different name dedicated for table level, or keep it like my current change?

@J-HowHuang J-HowHuang added metrics Related to metrics emission and collection query Related to query processing labels Jun 29, 2026
@codecov-commenter

codecov-commenter commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.80%. Comparing base (25e378b) to head (8de2972).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #18882   +/-   ##
=========================================
  Coverage     64.79%   64.80%           
  Complexity     1322     1322           
=========================================
  Files          3393     3393           
  Lines        211235   211334   +99     
  Branches      33206    33234   +28     
=========================================
+ Hits         136878   136951   +73     
- Misses        63303    63323   +20     
- Partials      11054    11060    +6     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.80% <100.00%> (+<0.01%) ⬆️
temurin 64.80% <100.00%> (+<0.01%) ⬆️
unittests 64.80% <100.00%> (+<0.01%) ⬆️
unittests1 57.00% <100.00%> (-0.02%) ⬇️
unittests2 37.17% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java Outdated
@J-HowHuang J-HowHuang requested a review from Jackie-Jiang June 29, 2026 21:05

@xiangfu0 xiangfu0 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.

Found a high-signal issue; see inline comment.

@Jackie-Jiang Jackie-Jiang 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

*/
public enum ServerMeter implements AbstractMetrics.Meter {
QUERIES("queries", true),
QUERIES_ON_TABLE("queries", false),

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.

(optional) I prefer TABLE_QUERIES, but completely optional

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think QUERIES_ON_TABLE makes sense as in "how many queries on table" versus "how many table queries"

@Jackie-Jiang Jackie-Jiang merged commit 91be61f into apache:master Jul 1, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metrics Related to metrics emission and collection query Related to query processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants