feat: add an in-process worker transport (no gRPC)#27
Draft
mdashti wants to merge 4 commits into
Draft
Conversation
The worker-protocol abstraction shipped only a gRPC implementation, so nothing exercised it without gRPC. This routes the three protocol methods straight to a co-located `Worker`, and is the seam a shared-memory transport plugs into. A worker resolves its own nested stage reads back through the same transport, so multi-stage queries stay in process.
A co-located transport (in-process worker, shared-memory mesh) speaks the same wire shape as gRPC, so the prost messages and the metrics codec have to build with the feature off and crate::proto must resolve either way. The plan-metrics collector and the receive-time stamp are the seams such a transport drives directly.
The benchmarks dev-dependency and a few `tonic`-importing test helpers re-unified gRPC into every test build, so nothing exercised the in-process transport with gRPC off. Moving the dataset suites into the benchmarks crate and gating the gRPC-coupled helpers lets `cargo test --no-default-features --lib` run the in-process transport, guarded by a new CI job.
deb340e to
02ff23b
Compare
Lets an embedder supply the dispatch bytes per stage instead of the coordinator encoding the plan it holds, whose exec-time state can be wrong to dispatch or not serializable through datafusion-proto.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes @gabotechs's
WorkerChannelabstraction (datafusion-contrib#512) usable by a non-gRPC transport, and adds the in-process reference transport that proves it. The base here is a mirror of hisabstract-grpctip, so the diff is just our additions.What
InProcessChannelResolver(src/protocol/in_process.rs): aChannelResolverbacked by a co-locatedWorker, routing the three protocol methods straight to it, with no gRPC, no IPC, and no serialization round-trip. The reference implementation of the protocol for a co-located worker.crate::proto,protocol::metrics_proto): a non-gRPC transport speaks the same wire shape but must build withgrpcoff, so only the tonic client/server stay gated.collect_plan_metrics_protos,set_received_time, theRemoteWorkUnitFeedTxstype.unit-test-no-grpcjob runscargo test --no-default-features --lib, which actually runs the in-process transport test with no gRPC. Making that compile needed decoupling thebenchmarksdev-dependency (it re-unifiedgrpcinto every test build), so the dataset suites moved to the benchmarks crate and the gRPC-coupledtest_utilsis gated behindgrpc.Why
datafusion-contrib#512 abstracts the worker protocol behind
WorkerChannel, but gRPC was still the only implementation and the proto sat behind thegrpcfeature, so nothing exercised the abstraction without gRPC. This is the smallest set of changes that makes it real: a transport that is not gRPC, building and tested withgrpcoff.Verified
cargo test --no-default-features --libruns the in-process transport test (a real distributedGROUP BYacross tasks) with no gRPC; clippy + check clean in both feature configs.Stack
The public embedder API an out-of-crate driver needs (shuffle routing, the metrics store,
prepare_in_process_plan) that datafusion-contrib#512 dropped is a separate PR on top of this (#29). The shared-memory transport is on top of that (#28), and its diff is onlysrc/shm/*— it plugs in with no core changes, which is the test that this abstraction is sufficient.