gortiz opened a new pull request, #20093:
URL: https://github.com/apache/pinot/pull/20093
## Summary
This PR makes a single-stage (v1) server stop building a response as soon as
it exceeds `maxServerResponseSizeBytes`. Before, the server built and
serialized the full response, and only then compared its size with the limit.
The PR also makes the query resource accountant sample the response every 1 MB
written, so a response with few large rows is visible to it.
This is the third of three steps:
1. #19906 (merged): paged builder sections and an exact-size `toBytes()`.
2. #19943 (merged): send the pages to Netty and gRPC without a contiguous
`byte[]`.
3. This PR: check the size limit and sample the accountant while the
response is built.
## Background
After #19906 and #19943, a large response no longer needs humongous arrays.
But the server still builds the whole response in memory before it rejects it:
- `QueryScheduler` compares `response.size()` with
`maxServerResponseSizeBytes` after `toDataBuffer()`. The code had a `TODO:
Perform this check sooner`.
- The callers of `DataTableBuilder` sample the accountant every 8192 rows. A
group-by with sketch aggregations can return a few thousand rows of more than
10 KB each. The accountant samples such a response only at row 0, so it does
not see tens of MB of allocations.
## Changes
```
QueryScheduler.processQueryAndSerializeResponse
└─ sets QueryExecutionContext.maxResponseSizeBytes from
maxServerResponseSizeBytes
└─ results block getDataTable()
└─ BaseDataTableBuilder.finishRow()
├─ size > limit → throw
ResponseSizeLimitExceededException
└─ every 1 MB written →
QueryThreadContext.checkTerminationAndSampleUsage()
```
- **`QueryExecutionContext`** gets `maxResponseSizeBytes`. The default is
`Integer.MAX_VALUE`, because the wire format stores section offsets as ints.
`QueryScheduler` sets it from the query option before it opens the
`QueryThreadContext`. Thus none of the places that create a builder change.
- **`BaseDataTableBuilder.finishRow()`** compares the bytes written so far
with the limit. The count is a lower bound of the serialized size: the fixed
size section, the variable size section, and in `DataTableBuilderV4` the new
string dictionary entries (4 bytes plus one byte per char). Thus the builder
never rejects a response that the old check accepted.
- **`ResponseSizeLimitExceededException`** (new) is a `QueryException` with
`QUERY_CANCELLATION`, the same code as the old check.
- **`QueryScheduler`** replies to both the early and the post-serialization
overflow through one method, `responseSizeExceeded()`. It logs the error,
increments `LARGE_QUERY_RESPONSE_SIZE_EXCEPTIONS` once, and sends the error
response. The post-serialization check stays, because it also counts the
header, the data schema and the metadata.
- **`DataTableBuilderV4`** replaces the deprecated `computeIntIfAbsent()`
with `getInt()` and `put()`, to count the new dictionary entries.
## Compatibility
- **Wire format and query options:** no change.
- **Error message:** an early overflow returns `Serialized query response
size of at least <n> exceeds threshold <limit> for requestId ...`. A
post-serialization overflow returns the same message as before: `Serialized
query response size <n> exceeds threshold <limit> for requestId ...`.
- **Metrics:** after an early overflow, `queryResponseSize` records the
bytes written until the builder stopped, because the full size is unknown.
- **Responses over 2 GB without a limit:** they now fail while they are
built, with `QUERY_CANCELLATION`, and increment
`largeQueryResponseSizeExceptions`. Before, they failed after the build with an
`INTERNAL` error and incremented `responseSerializationExceptions`.
- **gRPC:** the gRPC query server never applied
`maxServerResponseSizeBytes`, and it still does not. Its builders use the
default limit of 2 GB.
- **Timeouts and kills:** the builder now also checks termination every 1
MB. Thus a query that times out or that the accountant kills while it builds a
large response stops sooner.
## Tests
- `DataTableBuilderSizeLimitTest` (new): the limit on the fixed size
section, the variable size section and the string dictionary, with the exact
row that overflows. No limit without a query context. The cap of the limit to
`Integer.MAX_VALUE`. Sampling by bytes for large rows, and no sampling for
small tables.
- `QuerySchedulerResponseSizeTest` (new):
- A response within the limit.
- A response over the limit: the builder stops at row 13 of 1000. The
server replies with `QUERY_CANCELLATION` and records the metric once.
- A response whose rows fit but whose serialized form does not.
- No limit without the option.
- The existing `OfflineClusterIntegrationTest#testMaxServerResponseSize*`
tests expect `QUERY_CANCELLATION`, which does not change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
