gortiz opened a new pull request, #19906:
URL: https://github.com/apache/pinot/pull/19906
## Summary
This PR stops the single-stage (v1) server from building large responses in
arrays that grow by doubling. `DataTableBuilderV4` now writes the fixed and
variable size sections in 16 KB heap pages. `DataTableImplV4` reads these pages
directly, and `toBytes()` copies them once into one array of the exact final
size. The wire format does not change, and the broker does not change.
This is the first of three steps:
1. This PR: paged builder sections and an exact-size `toBytes()`. One
exact-size array replaces four arrays that grow or copy the full response.
2. Next: send the pages to Netty and gRPC without the final contiguous
`byte[]`.
3. Then: check `maxServerResponseSizeBytes` while the response is built, and
sample the query resource accountant by bytes written.
## Background
A G1 humongous object is any object of half a region or more: 8 MB with 16
MB regions. G1 allocates each one in contiguous free regions, and it reclaims
them only in some collections. Many large humongous allocations in a short time
can leave G1 with no free regions to evacuate into. Then a young GC fails to
evacuate and promotes live and dead young objects to the old generation.
A v1 group-by with many `OBJECT` intermediate results (for example
`DISTINCTCOUNTHLLPLUS`, about 11 KB per group) easily gives responses of 40 MB
to 80 MB per server. Before this PR, a response of size N went through these
allocations:
| Step | Allocation |
|---|---|
| `BaseDataTableBuilder` variable section (`ByteArrayOutputStream`) | grows
by doubling, up to 2N |
| `DataTableBuilderV4.build()` → `toByteArray()` | N (exact copy) |
| `DataTableImplV4.toBytes()` → new `ByteArrayOutputStream` | grows by
doubling, up to 2N |
| `toBytes()` → `toByteArray()` | N (exact copy) |
For N = 67 MB, the peak was about 268 MB of short-lived humongous arrays in
five allocations. After this PR, the only large array is the exact-size result
of `toBytes()`. The next PR removes it.
## Changes
- **`BaseDataTableBuilder`.** The two sections are `PagedPinotOutputStream`s
with `HeapPageAllocator.createSmall()` (16 KB pages, the same as the
multi-stage `DataBlockBuilder`). The offset into the variable section uses
`Math.toIntExact`, so a section larger than 2 GB fails instead of wrapping
around. The class Javadoc now states that the builder must not get more rows
after `build()`, because the built table shares its pages.
- **`DataTableBuilderV4.build()`.** It gives the pages to the table as
`DataBuffer` views. It does not copy them.
- **`DataTableImplV4`.**
- The sections are `DataBuffer`s, and every read uses an absolute offset.
This follows `BaseDataBlock`, but the array getters read element by element
with absolute offsets instead of with a stream, because a stream over a paged
buffer creates a view of every remaining page. The reads no longer move a
shared `ByteBuffer` position.
- The deserializing constructor wraps its section arrays in
`PinotByteBuffer`. It does not add copies.
- `getCustomObject` still returns a view. It returns a copy only when a
value crosses a page boundary. On a table built in the same process, the view
is read-only.
- `toBytes()` computes the final length first, then writes everything into
one array. The `RESPONSE_SER_CPU_TIME_NS` and
`RESPONSE_SER_MEM_ALLOCATED_BYTES` entries are reserved before the length is
computed and filled in after the copy. Both are `LONG` keys, so their encoded
length does not depend on the value.
- The constructor that took two `byte[]` sections now takes two
`DataBuffer`s. `DataTableBuilderV4` was its only caller.
- **`CompoundDataBuffer.getBufferIndex`.** It first checks the buffer of the
previous lookup, then does a binary search. Before, every backward lookup
scanned from the first buffer. A large table built in the broker merge path
(`GroupByDataTableReducer`, `AggregationDataTableReducer`) has thousands of
pages and is read in column and random order, so the scan made a full read
quadratic.
A builder now allocates two 16 KB pages up front, instead of two 32-byte
arrays. These are normal young-generation allocations.
## Tests
- `DataTableSerDeTest#testValuesSpanningPages` (new): `BYTES`, `INT_ARRAY`,
`STRING_ARRAY`, `BIG_DECIMAL_ARRAY` and `OBJECT` values of up to 40 KB, which
cross page boundaries. It also has null `OBJECT` values and an `INT` column
with a null bitmap. The test reads the built (paged) table,
`toDataOnlyDataTable()` of it, and the deserialized table.
- `DataTableSerDeTest#testAllDataTypesInOneSchema` now also reads the built
table before the round trip.
- `CompoundDataBufferTest#randomAccessAcrossBuffersOfDifferentSizes` (new):
random and backward reads across buffers of different sizes.
- All tests in `pinot-segment-spi`, `pinot-common` and `pinot-core` that
match `*DataBuffer*Test`, `*DataTable*Test`, `*DataBlock*Test`,
`*GroupBy*QueriesTest`, `*DistinctCount*QueriesTest`, `*NullHandling*Test`,
`*Selection*QueriesTest` and `AllNullQueriesTest` pass.
--
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]
