Add engine.execute() returning a PersistedQueryResult - #23114
Conversation
012f980 to
3d4580f
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds experimental ChangesPersisted execute() pipeline
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
TomAugspurger
left a comment
There was a problem hiding this comment.
Should we have some kind of base .execute() method on StreamingEngine? Same for drop_persisted?
Co-authored-by: Tom Augspurger <tom.augspurger88@gmail.com>
|
|
||
| ## Chaining into another `execute()` | ||
|
|
||
| `result.lazy()` is an ordinary `LazyFrame`, so you can feed it (or further work |
There was a problem hiding this comment.
Note, it's not really an ordinary LazyFrame because you can't collect it on the CPU, no?
There was a problem hiding this comment.
good point, updated to:
`result.lazy()` supports the usual `LazyFrame` API, so you can add further
operations and pass the resulting query directly to `engine.execute()` without
collecting it first.
Only materialization is special. Like any persisted result, it must run on the
engine that produced it, as described above, and cannot be collected directly
on the host. The query executes on the GPU, and its output remains there as a
new persisted result. This makes it useful for building multi-step pipelines
without transferring intermediate results to host memory:
| `LazyFrame` twice, or to use it in an operation that reads it multiple times | ||
| (such as a self-join), raises a `RuntimeError`. Call `engine.execute()` again |
There was a problem hiding this comment.
This is bad (the self-join thing). I guess the problem is that Polar's common subplan elimination doesn't see that these are the same thing twice?
There was a problem hiding this comment.
Yes, CSE will sometimes collapse the duplicate scan, so a self-join can happen to work, but not in the general case. Support for repeated reads is tracked in #23115.
| # The process-global set of per-engine stores, keyed by uid | ||
| _stores: dict[str, RankLocalStore] = {} |
There was a problem hiding this comment.
Question: can we somehow have this thing tied to an engine, rather than being process-global?
There was a problem hiding this comment.
I don't think so. I initially explored several ways to keep the store on the engine, but persisted partitions are loaded by PersistedSource, which runs during query execution in the worker deep inside the Polars IR.
That callback receives only the serialized scan arguments, the engine uid and query_id. It has no reference to the engine object, which lives in the client process.
Co-authored-by: Lawrence Mitchell <wence@gmx.li>
Co-authored-by: Lawrence Mitchell <wence@gmx.li>
TomAugspurger
left a comment
There was a problem hiding this comment.
I think all my questions have been addressed.
|
/merge |
…ction DataFrame.from_table lost its num_rows parameter in NVIDIA#23234 (row counts are now inferred from the pylibcudf table, which carries them even for zero-column tables), but the duplicated-output path added in NVIDIA#23114 still passed num_rows=0, breaking mypy on every PR and raising TypeError at runtime on that path. empty_like already produces a 0-row table (including for zero-column inputs), so the argument was redundant.
…utput path (#23303) `DataFrame.from_table` lost its `num_rows` parameter in #23234 (row counts are now inferred from the pylibcudf table, which carries them even for zero-column tables), but the duplicated-output path added in #23114 still passes `num_rows=0`. The two PRs merged around the same time, so this surfaced only after both landed: mypy now fails on every PR's `check-style` job (`engine/core.py:840: Unexpected keyword argument "num_rows"`), and the path would raise `TypeError` at runtime. `plc.copying.empty_like` already produces a 0-row table (including for zero-column inputs, verified), so dropping the argument preserves the intended "freshly-allocated empty same-schema frame" semantics exactly. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) - Lawrence Mitchell (https://github.com/wence-) URL: #23303
…utput path (NVIDIA#23303) `DataFrame.from_table` lost its `num_rows` parameter in NVIDIA#23234 (row counts are now inferred from the pylibcudf table, which carries them even for zero-column tables), but the duplicated-output path added in NVIDIA#23114 still passes `num_rows=0`. The two PRs merged around the same time, so this surfaced only after both landed: mypy now fails on every PR's `check-style` job (`engine/core.py:840: Unexpected keyword argument "num_rows"`), and the path would raise `TypeError` at runtime. `plc.copying.empty_like` already produces a 0-row table (including for zero-column inputs, verified), so dropping the argument preserves the intended "freshly-allocated empty same-schema frame" semantics exactly. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) - Lawrence Mitchell (https://github.com/wence-) URL: NVIDIA#23303
Description
This PR adds an
execute()API to the streaming cudf-polars engines (SPMD, Dask, and Ray) that returns aPersistedQueryResult. Instead of gathering the output to the client as a singleDataFrame, each rank's partition stays GPU-resident in the process that produced it.PersistedQueryResult.lazy()converts the persisted result back into aLazyFrame, allowing additional queries to be chained without an intermediate host round-trip:This provides the foundation for supporting Polars'
LazyFrame.execute()/QueryResultAPIs on GPU engines while keeping intermediate results GPU-resident and rank-local.The implementation is built entirely on the existing
RankAwareSourceI/O-plugin infrastructure. Each rank's output partition is exposed through a registered scan source, and workers read only their local partition when the result is collected. Reads are move-on-read, so aPersistedQueryResultcan only be collected once (see the design note).Limitation
We expose this as
engine.execute()because there is currently no way to hook into Polars' ownpl.LazyFrame.execute(engine=...)call. This is one of the Polars gaps tracked in #22917.The Approach
The lifecycle of a persisted result spans three phases. Each rank keeps its own output partition GPU-resident in a process-local store keyed by
(query_id, rank)(seecudf_polars.engine.rank_local_store), so nothing crosses the process boundary until the caller explicitly collects.Execute (
engine.execute(lf)->PersistedQueryResult):lfto IR, creates aquery_id, and asks the backend to execute the query on each rank.PersistedQueryResultreferencing the producing ranks.Collect (
PersistedQueryResult.lazy()->collect(engine=...)):PersistedQueryResult.lazy()exposes the stored partitions as a Polars IO-pluginLazyFrameso downstream query nodes can chain onto it normally.Release (GC finalizer /
PersistedQueryResult.release()/ context manager):weakref.finalizethat callsPersistedBackend.drop_persisted, broadcasting a storedropfor thequery_idto every rank. It is idempotent and never raises, so a collected, released, or reset result all clean up safely.