mdreizin commented on PR #40983: URL: https://github.com/apache/beam/pull/40983#issuecomment-6666559794
@goutamadwant thanks, this works well functionally. I tested it with parquet-go against real GCS objects, and generation pinning, gzip rejection, EOF handling and close all behave as expected. A single `ReadAt` over a whole 38 MB object matches `OpenRead` throughput (~640 ms vs ~610 ms), and connections are reused across `ReadAt` calls. One data point on access patterns, since each `ReadAt` is one range request. Parquet background for the numbers below: a Parquet file is split horizontally into row groups, each a contiguous block of rows. Inside a row group, each column's values are stored together as a column chunk. The footer records the byte offset and size of every column chunk, so a reader can fetch one row group, or only some columns, without touching the rest of the file. parquet-go reads column chunk by column chunk, so a naive positional read issues one or more small reads per column chunk. Full scan of a wide file (38 MB, 137,649 rows, 219 columns, 7 row groups of 1.2-9.7 MB, ~25 KB column chunks), from a workstation outside GCP: | Read pattern | Time | Requests | Bytes fetched | File bytes held at once | |---|---|---|---|---| | `OpenRead`, decode from memory | ~1.1-1.4 s | 1 | 1.00x | 38.5 MB (whole file) | | `OpenSeekable` + `parquet.OpenFile`, `ReadModeAsync` | ~5-7 s | 2,908 (up to 219 in flight) | 1.52x | n/a | | `OpenSeekable` + `parquet.OpenFile`, `ReadModeSync` | ~4 min | 2,908 | 1.52x | n/a | | `OpenSeekable`, one `ReadAt` per row group, 1 prefetched | ~2.2-2.9 s | 10 | 1.00x | ≤ 10.9 MB (2 row groups) | | `OpenSeekable`, one `ReadAt` per row group, 3 prefetched | ~1.4-2.4 s | 10 | 1.00x | ≤ 21.3 MB (4 row groups) | | `OpenSeekable`, one `ReadAt` per row group, all prefetched | ~1.3-1.7 s | 10 | 1.00x | 38.5 MB (whole file) | The slow rows come from parquet-go's access pattern, not from this PR: 97% of its `ReadAt` calls were under 64 KB regardless of `parquet.ReadBufferSize`, and that buffer is allocated once per column, so large values add up on wide files. Coalescing per row group brings it to 10 requests with nothing fetched twice. What's left is per-request latency (~150-300 ms per request from outside GCP, almost regardless of size). With one row group prefetched the requests run mostly one after another; keeping a few in flight hides it, at the cost of holding those row groups in memory. I think the split in this PR is right: coalescing and request scheduling belong to the caller, as discussed in #39176. Two small suggestions: - A sentence on `SeekableOpener` / `ReadableFile.OpenSeekable` saying that on object stores each `ReadAt` maps to one request, so callers should coalesce small reads and keep a few in flight to hide latency. - Keeping `ReaderAtCloser` exactly `io.ReaderAt` + `io.Closer`, as it is now. Batched reads or read-ahead hints can then be added later as separate optional interfaces, the same way the Go filesystem package already does for `Copier` and `Renamer`, without breaking existing implementations. -- 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]
