SteNicholas opened a new issue, #419: URL: https://github.com/apache/paimon-cpp/issues/419
## Search before asking - [x] I searched in the [issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar. ## Motivation #289 overlapped the first read of a data file with the consumption of the previous one, but only on the merge-on-read path: it added `Warmup()` to `FileBatchReader` and `KeyValueRecordReader` and calls it from `ConcatKeyValueRecordReader` and `LoserTree`. The read paths that do not merge concatenate their data files through `ConcatBatchReader` instead, which never calls `Warmup()`, so they still issue the first remote read of every file serially, only after the previous file has reached EOF. The most direct case is append compaction. `AppendOnlyFileStoreWrite::CreateFilesReader` builds its read context with prefetch enabled and leaves the read-ahead cache and the warmup level at their defaults (enabled and `WarmupLevel::RAW`), then reads the files to compact through `RawFileSplitRead::CreateReader`, which is one `ConcatBatchReader` over the files' `FileBatchReader` stacks. `CompactRewrite` drains that reader sequentially on a single thread. Every file's prefetch reader is already built and has its read schema set before the first batch is read, because `CreateRawFileReadersWithMeta` builds the readers of all files up front, so each of them could be warmed as is; nothing ever asks. A compaction that rewrites many small files, which is the rewrite append compaction exists for, therefore waits for one remote round trip at every file boundary. The same `ConcatBatchReader` concatenates `FileBatchReader`s on other paths too, which would get the same overlap: - append-table batch reads, through the same `RawFileSplitRead::CreateReader` that compaction uses; - raw-convertible primary-key splits read without merging, through the raw path of `MergeFileSplitRead`; - the blob files of one data file in `DataEvolutionSplitRead`, where it is a no-op in practice because blob does not go through the prefetch reader. ## Solution Warm one file ahead in `ConcatBatchReader`, the same way `ConcatKeyValueRecordReader` does since #289. - `ConcatBatchReader` holds its children as `BatchReader`, while `Warmup()` is declared on `FileBatchReader`. Resolve each child's `FileBatchReader*` once in the constructor, `nullptr` for a child that is not one, which is the same boundary cast `KeyValueDataFileRecordReader::Warmup()` makes, rather than casting on every batch. Children that are not `FileBatchReader`s keep today's behavior: the per-split readers concatenated by `AppendOnlyTableRead` and `KeyValueTableRead`, the fallback and audit-log readers, the sort-merge sections of `MergeFileSplitRead`, and the realtime commit readers. - In `NextBatchWithBitmap()`, warm the current child and `kWarmupLookahead = 1` child after it before reading, as `ConcatKeyValueRecordReader::NextBatch()` does. `Warmup()` is idempotent, so on readers that are already warm this costs one virtual call and one pointer test per child per batch. `NextBatch()` goes through `NextBatchWithBitmap()` and needs no change of its own. - Do not add `Warmup()` to the public `BatchReader`. That would change the vtable of an exported class, and it would only be needed to warm a `ConcatBatchReader` from the outside, which no caller does today. Tests go into the existing `concat_batch_reader_test.cpp`, using `MockFileBatchReader::GetWarmupCount()`: reading child `i` warms child `i + 1` and nothing beyond it, a child that is not a `FileBatchReader` is skipped, and the output is unchanged. ## Anything else? Warming changes when a file's first bytes are fetched, never what is read: ordering, filtering, deletion vectors and metrics are untouched. As in #289 there is no new configuration; the existing `ReadContextBuilder::SetWarmupLevel()` and `SetReadAheadCacheEnabled()` already control it. It only has an effect for formats that go through the prefetch reader, so avro, blob, lance and mosaic files are unaffected. The costs, stated up front: - Memory. Under `RAW` the next file's ranges are fetched through its read-ahead cache, up to `CacheConfig` `pre_buffer_limit` (256 MiB by default), so while one file is being consumed the next one can hold up to min(its data size, 256 MiB) on top of it. Append compaction builds its own `ReadContext`, and the warmup level is not a table option, so compaction cannot opt out. This is the trade-off #289 and #365 already accepted for merge reads; if it turns out to matter for compaction, `CreateFilesReader` can set the level explicitly in a follow-up rather than adding a knob here. - Gain. Warming saves the first-read round trip at each file boundary. It matters for compactions and splits made of many small files, and is marginal for a few large ones. - Early stop. A read that stops early, for example under a LIMIT, may already have warmed a file it never reads. `ReadAheadCache::ReleasePrefetchBuffers()` waits for dispatched fetches before freeing their buffers, so closing such a read, or cancelling a compaction, can also wait for the warmed file's in-flight fetches. The merge path has the same behavior today. Warming still does not cross a split boundary, because the outer `ConcatBatchReader` of `AppendOnlyTableRead` and `KeyValueTableRead` concatenates per-split readers that are not `FileBatchReader`s. That is left as a follow-up. For comparison, Velox preloads whole splits ahead of the consumer (`max_split_preload_per_driver`, 2 by default) and opens the file and its reader on an IO executor. Paimon already builds every reader of a split up front, so within a split only the first data fetch remains to be overlapped, and that is what this change covers. ## Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- 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]
