JingsongLi commented on PR #8807: URL: https://github.com/apache/paimon/pull/8807#issuecomment-5229857065
Thanks for the follow-up changes. After reviewing the current head (`47d9cac`), I think the implementation can be simplified by making the split pipeline the single owner of concurrency and readahead: 1. **Reuse the existing scan cost model.** Use `source.split.target-size` and `source.split.open-file-cost` to estimate work instead of introducing another set of reader-side projection/range estimates and fixed 64/256 MiB thresholds. 2. **Keep coordination state proportional to active workers.** Use an `O(workers)` sliding window: submit only the active splits, then submit the next split when one finishes. This removes the per-split `Semaphore`, eager enqueue of all tasks, and potentially unbounded `pending` state. 3. **Avoid nested readahead.** When the outer split pipeline is enabled, reduce or disable the inner PyArrow Scanner `batch_readahead` / `use_threads` / Parquet `pre_buffer`, or account for them in one shared count/byte budget. Otherwise, the outer "one batch per active split" slot does not bound the batches decoded and retained inside each scanner. 4. **Use one default policy across APIs.** `to_arrow_batch_reader`, `to_arrow`, and `to_pandas` should use the same adaptive worker policy unless there is benchmark evidence that materializing reads need different defaults. Explicit `parallelism` and `read.parallelism` should remain authoritative overrides. 5. **Keep the automatic policy small and predictable.** Stay serial for small local input; use a modest active window, for example 4, for multiple remote splits where open/RTT cost dominates; let explicit configuration request higher concurrency. The exact thresholds should come from benchmarks covering time to first batch, throughput, and peak RSS. In short: **split planning should determine the amount of work, the reader should maintain only a bounded active window, and PyArrow should not independently add another concurrency layer.** This would make the implementation smaller while also addressing the current `O(total splits)` coordination overhead and hidden nested-prefetch memory amplification. -- 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]
