Add metrics for serialized query response size in QueryScheduler#17710
Add metrics for serialized query response size in QueryScheduler#17710gortiz wants to merge 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new server-side metric to track serialized query response sizes, and refactors QueryScheduler to record size and handle oversized responses with a dedicated exception meter.
Changes:
- Introduced
ServerMeter.QUERY_RESPONSE_SIZEto track serialized response size (bytes). - Updated
QuerySchedulerto record response size and only emit oversize exception/logging when configured thresholds are exceeded.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java |
Records response size and refines oversized-response handling logic. |
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java |
Adds the new QUERY_RESPONSE_SIZE meter definition and description. |
| if (maxResponseSizeBytes != null && responseBytes != null) { | ||
| int responseSizeBytes = responseBytes.length; | ||
| String tableNameWithType = queryRequest.getTableNameWithType(); | ||
| _serverMetrics.addMeteredTableValue(tableNameWithType, ServerMeter.QUERY_RESPONSE_SIZE, responseSizeBytes); | ||
| if (responseSizeBytes > maxResponseSizeBytes) { |
There was a problem hiding this comment.
QUERY_RESPONSE_SIZE is only recorded when maxResponseSizeBytes != null, so queries without this option configured won't emit the new metric. If the goal is to track response sizes for all queries, record the metric whenever responseBytes != null, and only guard the threshold/exception handling with the maxResponseSizeBytes check.
| if (maxResponseSizeBytes != null && responseBytes != null) { | |
| int responseSizeBytes = responseBytes.length; | |
| String tableNameWithType = queryRequest.getTableNameWithType(); | |
| _serverMetrics.addMeteredTableValue(tableNameWithType, ServerMeter.QUERY_RESPONSE_SIZE, responseSizeBytes); | |
| if (responseSizeBytes > maxResponseSizeBytes) { | |
| if (responseBytes != null) { | |
| int responseSizeBytes = responseBytes.length; | |
| String tableNameWithType = queryRequest.getTableNameWithType(); | |
| _serverMetrics.addMeteredTableValue(tableNameWithType, ServerMeter.QUERY_RESPONSE_SIZE, responseSizeBytes); | |
| if (maxResponseSizeBytes != null && responseSizeBytes > maxResponseSizeBytes) { |
There was a problem hiding this comment.
+1, don't we want to record the response size for all queries?
| * Size of the serialized response sent from server to broker in bytes. | ||
| */ | ||
| QUERY_RESPONSE_SIZE("bytes", false, "Size of the serialized response sent from server to broker in bytes"), |
There was a problem hiding this comment.
The QUERY_RESPONSE_SIZE description says the metric is for the response "sent from server to broker", but in QueryScheduler it is recorded immediately after the first serialization and can represent a response that is later replaced with an error response when it exceeds maxResponseSizeBytes. Either move the metric recording to after the final responseBytes is determined, or adjust the description to clarify what is being measured.
| * Size of the serialized response sent from server to broker in bytes. | |
| */ | |
| QUERY_RESPONSE_SIZE("bytes", false, "Size of the serialized response sent from server to broker in bytes"), | |
| * Size of the initially serialized query response in bytes. | |
| * Note: This may differ from the final response actually sent to the broker if the response | |
| * is later replaced (for example, when exceeding the configured max response size). | |
| */ | |
| QUERY_RESPONSE_SIZE("bytes", false, | |
| "Size of the initially serialized query response in bytes (may differ from final response sent to broker)"), |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17710 +/- ##
============================================
- Coverage 63.23% 63.22% -0.01%
+ Complexity 1502 1501 -1
============================================
Files 3179 3179
Lines 190710 190715 +5
Branches 29153 29154 +1
============================================
- Hits 120597 120589 -8
Misses 60746 60746
- Partials 9367 9380 +13
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:
|
3 participants
This pull request adds new metrics tracking for server query response sizes. The main focus is on recording the size of each serialized query responsee.
Metrics enhancements:
QUERY_RESPONSE_SIZEto theServerMeterenum to track the size (in bytes) of serialized server-to-broker responses.QuerySchedulerto record the size of each query response using the newQUERY_RESPONSE_SIZEmeter for the corresponding table.Large response handling improvements:
QuerySchedulerto always record the response size, and only log an exception and increment theLARGE_QUERY_RESPONSE_SIZE_EXCEPTIONSmeter when the response exceeds the configured maximum size. [1] [2]