Gaurav598 commented on PR #40496: URL: https://github.com/apache/beam/pull/40496#issuecomment-6177994488
Hi @claudevdm, thanks for taking the time to review this! I understand the DaskRunner is experimental and may not be familiar territory, so here's a quick overview of the change: - **Root cause:** `DaskBagWindowedIterator` eagerly materializes the entire side-input bag, potentially causing OOM for large side inputs. - **Approach:** The new `--dask_lazy_side_inputs` flag enables partition-by-partition evaluation using Dask's `to_delayed()`, while preserving the existing behavior by default. - **Worker safety:** Nested computations inside Dask workers use `worker_client()` to avoid blocking the worker's scheduling slot, including in single-worker/single-thread configurations. - **Validation:** Added focused and distributed regression tests, and the latest CI checks are now passing, including Codecov patch coverage at 80.95%. The PR description also documents the remaining limitations and trade-offs, particularly serial partition evaluation and `AsList` materialization. Happy to clarify any part of the implementation or provide additional context if helpful. No rush, and thanks again for reviewing! -- 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]
