Add pinot_server_mse_queries server metrics#18287
Merged
yashmayya merged 4 commits intoApr 23, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18287 +/- ##
============================================
+ Coverage 63.60% 63.62% +0.01%
Complexity 1659 1659
============================================
Files 3246 3246
Lines 197510 197512 +2
Branches 30578 30578
============================================
+ Hits 125620 125658 +38
+ Misses 61845 61810 -35
+ Partials 10045 10044 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@Jackie-Jiang @yashmayya can one of you take a look at this? |
xiangfu0
added
metrics
yashmayya
reviewed
Apr 22, 2026
Contributor
yashmayya
left a comment
yashmayya
left a comment
There was a problem hiding this comment.
Thanks @timothy-e, I've left a few minor comments.
| /// Calls {@link GrpcQueryServer#submit} directly (bypassing gRPC transport) and verifies that | ||
| /// {@link ServerMeter#MSE_QUERIES} is never touched by the SSE path. | ||
| @Test | ||
| public void testMseQueriesNotIncrementedBySsePath() { |
Contributor
There was a problem hiding this comment.
Do we really need this test and test class? Feels low value to me
Contributor
Author
There was a problem hiding this comment.
It wasn't super obvious to me that the code path pinot-query-runtime/src/main/java/org/apache/pinot/query/service/server/QueryServer.java is strictly MSE only. I knew MSE used GRPC, and I wanted to make sure that my change didn't erroneously affect other GRPC paths. I could do that with a manual test, but a unit test helps guarantee that if we ever refactor GRPC code to share more code, we won't start counting them as MSE requests.
Contributor
There was a problem hiding this comment.
Everything in pinot-query-planner / pinot-query-runtime is MSE only. Maybe we could do a better job of documenting that.
Contributor
Author
|
@yashmayya addressed comments - happy to remove the GRPC metric test if you still feel it provides little value. |
yashmayya
approved these changes
Apr 23, 2026
Contributor
yashmayya
left a comment
yashmayya
left a comment
There was a problem hiding this comment.
happy to remove the GRPC metric test if you still feel it provides little value
Yeah IMO that test isn't really relevant, let's remove it. Everything else looks good though, thanks for the patch!
| /// Calls {@link GrpcQueryServer#submit} directly (bypassing gRPC transport) and verifies that | ||
| /// {@link ServerMeter#MSE_QUERIES} is never touched by the SSE path. | ||
| @Test | ||
| public void testMseQueriesNotIncrementedBySsePath() { |
Contributor
There was a problem hiding this comment.
Everything in pinot-query-planner / pinot-query-runtime is MSE only. Maybe we could do a better job of documenting that.
Adds a new ServerMeter enum values that fire exactly once per incoming Worker.QueryRequest, giving operators a per-server MSE query count independent of stage/worker parallelism. In MSE, the broker sends a single Worker.QueryRequest to each server it dispatches to, and that single request contains all the StagePlans assigned to that server — whether those are leaf-stage plans (reading segments), intermediate-stage plans (aggregating/joining), or both. Therefore, `MSE_QUERIES` increments by exactly 1 regardless of how many stages (leaf / intermediate) are packed into that single QueryRequest. If a server happens to be assigned both a leaf stage and an intermediate stage for the same query, it still only receives one Worker.QueryRequest from the broker, and the counter goes up by 1. Matches the semantics of * [`QUERIES`](https://stripe.sourcegraphcloud.com/r/stripe-private-oss-forks/pinot/-/blob/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java?L29) that exists for SSE. [We increment SSE queries before deserializing](https://stripe.sourcegraphcloud.com/r/stripe-private-oss-forks/pinot/-/blob/pinot-core/src/main/java/org/apache/pinot/core/transport/InstanceRequestHandler.java?L124). * [`GRPC_QUERIES`](https://stripe.sourcegraphcloud.com/r/stripe-private-oss-forks/pinot/-/blob/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java?L128) that exists for SSE GRPC requests. [We increment GRPC SSE queries before deserializing](https://stripe.sourcegraphcloud.com/r/stripe-private-oss-forks/pinot/-/blob/pinot-core/src/main/java/org/apache/pinot/core/transport/grpc/GrpcQueryServer.java?L222). There was no MSE equivalent for a per-server query counter. We have some [broker-level metrics](https://g-8916660cfe.grafana-workspace.us-west-2.amazonaws.com/d/DltsoWuVk/pinot-components?orgId=1&var-datasource=zb219lV4k&var-min_interval=$__auto_interval_min_interval&var-host_cluster=All&var-host_type=All&var-pinot_cluster=rad-canary&var-pinot_tenant=All&var-table=All&var-table_type=All&var-pool_name=.%2A&var-pool_number=.%2A&from=1776105056507&to=1776105251812&viewPanel=185&editPanel=185), but for adaptive routing, we want distribution details. cc stripe-private-oss-forks/pinot-reviewers r? Better debugging of adaptive routing. [STREAMANALYTICS-4419](https://jira.corp.stripe.com/browse/STREAMANALYTICS-4419) [Deployed to rad-canary](https://amp.qa.corp.stripe.com/deploy/qa-deploy1.pdx.deploy.stripe.net%2Fdeploy_hczIi39pS-WRvHNRJp1j8A) in QA. Saw [`pinot_server_mse_queries`](https://g-8916660cfe.grafana-workspace.us-west-2.amazonaws.com/explore?schemaVersion=1&panes=%7B%22ik1%22:%7B%22datasource%22:%22zb219lV4k%22,%22queries%22:%5B%7B%22datasource%22:%7B%22type%22:%22prometheus%22,%22uid%22:%22zb219lV4k%22%7D,%22editorMode%22:%22code%22,%22expr%22:%22topk%28200,%20rate%28pinot_server_mse_queries%7Bhost_env%3D~%5C%22qa%5C%22,pinot_cluster%3D~%5C%22rad-canary%5C%22,pinot_tenant%3D~%5C%22long-lived-a%5C%22,host_cluster%3D~%5C%22northwest%5C%22%7D%5B$__rate_interval%5D%29%29%22,%22format%22:%22time_series%22,%22hide%22:false,%22instant%22:false,%22legendFormat%22:%22V2%20%7C%20%7B%7Bpinot_cluster%7D%7D%20%7C%20%7B%7Bhost%7D%7D%20%7C%20%7B%7Bpinot_tenant%7D%7D%20%20%7C%20%7B%7Bhost%7D%7D%22,%22refId%22:%22A%22,%22interval%22:%22%22%7D%5D,%22range%22:%7B%22from%22:%221776179346734%22,%22to%22:%221776180345904%22%7D%7D%7D&orgId=1) produce values. Stripe-Original-Repo: stripe-private-oss-forks/pinot Stripe-Monotonic-Timestamp: v2/2026-04-14T22:34:02Z/0 Stripe-Original-PR: https://git.corp.stripe.com/stripe-private-oss-forks/pinot/pull/593
Committed-By-Agent: claude
Committed-By-Agent: claude
timothy-e
force-pushed
the
timothye-mse-queries-server-metrics
branch
from
c542639 to
d3ec521
Compare
xiangfu0
pushed a commit
to pinot-contrib/pinot-docs
that referenced
this pull request
Apr 23, 2026
Documents the new MSE_QUERIES metric added by apache/pinot#18287
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new ServerMeter enum values that fire exactly once per incoming Worker.QueryRequest, giving operators a per-server MSE query count independent of stage/worker parallelism. This will help debug adaptive routing for MSE.
In MSE, the broker sends a single Worker.QueryRequest to each server it dispatches to, and that single request contains all the StagePlans assigned to that server — whether those are leaf-stage plans (reading segments), intermediate-stage plans (aggregating/joining), or both. Therefore,
MSE_QUERIESincrements by exactly 1 regardless of how many stages (leaf / intermediate) are packed into that single QueryRequest. If a server happens to be assigned both a leaf stage and an intermediate stage for the same query, it still only receives one Worker.QueryRequest from the broker, and the counter goes up by 1.Matches the semantics of
QUERIESthat exists for SSE. We increment SSE queries before deserializing.GRPC_QUERIESthat exists for SSE GRPC requests. We increment GRPC SSE queries before deserializing.There was no MSE equivalent for a per-server query counter, only broker-level stats.
To test, we deployed to an internal Stripe cluster and saw that the metric produces values that make sense.