Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ jobs:
- uses: ./.github/actions/setup
- run: cargo test --features integration

# Runs the whole lib unit suite with `grpc` off, the proof that the abstraction builds and runs
# without gRPC. `lfs: true` because some planner unit tests load parquet testdata.
unit-test-no-grpc:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
lfs: true
- uses: ./.github/actions/setup
- run: cargo test --no-default-features --lib

tpch-correctness-test:
runs-on: ubuntu-latest
strategy:
Expand All @@ -50,7 +61,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/setup
- run: cargo test --features tpch --test tpch_correctness_test
- run: cargo test -p datafusion-distributed-benchmarks --features tpch --test tpch_correctness_test
env:
ADAPTIVE: ${{ matrix.planning_mode == 'adaptive' }}

Expand All @@ -59,7 +70,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/setup
- run: cargo test --features tpch --test tpch_plans_test
- run: cargo test -p datafusion-distributed-benchmarks --features tpch --test tpch_plans_test

tpcds-correctness-test:
runs-on: ubuntu-latest
Expand All @@ -73,9 +84,9 @@ jobs:
- uses: ./.github/actions/setup
- uses: actions/cache@v4
with:
path: testdata/tpcds/main.zip
path: benchmarks/testdata/tpcds/main.zip
key: "main.zip"
- run: cargo test --features tpcds --test tpcds_correctness_test shard${{ matrix.shard }}
- run: cargo test -p datafusion-distributed-benchmarks --features tpcds --test tpcds_correctness_test shard${{ matrix.shard }}
env:
ADAPTIVE: ${{ matrix.planning_mode == 'adaptive' }}

Expand All @@ -86,9 +97,9 @@ jobs:
- uses: ./.github/actions/setup
- uses: actions/cache@v4
with:
path: testdata/tpcds/main.zip
path: benchmarks/testdata/tpcds/main.zip
key: "main.zip"
- run: cargo test --features tpcds --test tpcds_plans_test
- run: cargo test -p datafusion-distributed-benchmarks --features tpcds --test tpcds_plans_test

clickbench-correctness-test:
runs-on: ubuntu-latest
Expand All @@ -101,9 +112,9 @@ jobs:
- uses: ./.github/actions/setup
- uses: actions/cache@v4
with:
path: testdata/clickbench/
path: benchmarks/testdata/clickbench/
key: "data"
- run: cargo test --features clickbench --test clickbench_correctness_test
- run: cargo test -p datafusion-distributed-benchmarks --features clickbench --test clickbench_correctness_test
env:
ADAPTIVE: ${{ matrix.planning_mode == 'adaptive' }}

Expand All @@ -114,9 +125,9 @@ jobs:
- uses: ./.github/actions/setup
- uses: actions/cache@v4
with:
path: testdata/clickbench/
path: benchmarks/testdata/clickbench/
key: "data"
- run: cargo test --features clickbench --test clickbench_plans_test
- run: cargo test -p datafusion-distributed-benchmarks --features clickbench --test clickbench_plans_test

format-check:
runs-on: ubuntu-latest
Expand Down
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 39 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ sysinfo = { version = "0.30", optional = true }
sketches-ddsketch = { version = "0.3", features = ["use_serde"] }
num-traits = "0.2"
bincode = "1"
tonic-prost = "0.14.2"
tonic-prost = { version = "0.14.2", optional = true }

# grpc-specific features
arrow-flight = { version = "58", optional = true }
Expand All @@ -68,21 +68,17 @@ grpc = [
"arrow-select",
"arrow-ipc",
"tonic",
"tonic-prost",
"tower"
]

integration = ["insta", "parquet", "arrow", "hyper-util", "grpc"]

system-metrics = ["sysinfo"]

tpch = ["integration"]
tpcds = ["integration"]
clickbench = ["integration"]
slow-tests = []
sysinfo = ["dep:sysinfo"]

[dev-dependencies]
datafusion-distributed-benchmarks = { path = "benchmarks" }
structopt = "0.3"
insta = { version = "1.46.0", features = ["filters"] }
parquet = "58"
Expand All @@ -92,6 +88,43 @@ hyper-util = "0.1.16"
pretty_assertions = "1.4"
test-case = "3.3.1"

# Every example drives the gRPC transport, so a no-grpc build skips them.
[[example]]
name = "custom_distributed_partial_reduction_tree"
required-features = ["integration"]

[[example]]
name = "custom_execution_plan"
required-features = ["integration"]

[[example]]
name = "custom_worker_url_routing"
required-features = ["integration"]

[[example]]
name = "work_unit_feed"
required-features = ["integration"]

[[example]]
name = "in_memory_cluster"
required-features = ["grpc"]

[[example]]
name = "localhost_run"
required-features = ["grpc"]

[[example]]
name = "localhost_versioned_run"
required-features = ["grpc"]

[[example]]
name = "localhost_worker"
required-features = ["grpc"]

[[example]]
name = "localhost_versioned_worker"
required-features = ["grpc"]

[workspace.lints.clippy]
disallowed_types = "deny"
disallowed-methods = "deny"
11 changes: 11 additions & 0 deletions benchmarks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,20 @@ aws-sdk-ec2 = "1"
openssl = { version = "0.10", features = ["vendored"] } # Keep this. Necessary for the remote benchmarks worker.
mimalloc = "0.1"

[features]
# Gates for the dataset test suites under `tests/`. They live here rather than in the library
# because, as a dev-dependency of the library, this crate re-enables `grpc` on every test build
# through feature unification, which made the no-grpc config untestable.
tpch = []
tpcds = []
clickbench = []
slow-tests = []

[dev-dependencies]
criterion = "0.5"
sysinfo = "0.30"
pretty_assertions = "1.4"
test-case = "3.3.1"

[build-dependencies]
built = { version = "0.8", features = ["git2", "chrono"] }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "clickbench", test))]
#[cfg(all(feature = "clickbench", test))]
mod tests {
use datafusion::arrow::array::RecordBatch;
use datafusion::common::plan_err;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "clickbench", test))]
#[cfg(all(feature = "clickbench", test))]
mod tests {
use datafusion::error::Result;
use datafusion_distributed::test_utils::in_memory_channel_resolver::start_in_memory_context;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "tpch", test))]
#[cfg(all(feature = "tpch", test))]
mod tests {
use datafusion::common::instant::Instant;
use datafusion::error::Result;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "tpcds", test))]
#[cfg(all(feature = "tpcds", test))]
mod tests {
use datafusion::arrow::array::RecordBatch;
use datafusion::common::plan_err;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "tpcds", test))]
#[cfg(all(feature = "tpcds", test))]
mod tests {
use datafusion::error::Result;
use datafusion_distributed::test_utils::in_memory_channel_resolver::start_in_memory_context;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "tpch", test))]
#[cfg(all(feature = "tpch", test))]
mod tests {
use datafusion::physical_plan::execute_stream;
use datafusion::prelude::SessionContext;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(all(feature = "integration", feature = "tpch", test))]
#[cfg(all(feature = "tpch", test))]
mod tests {
use datafusion_distributed::test_utils::in_memory_channel_resolver::start_in_memory_context;
use datafusion_distributed::{
Expand Down
2 changes: 1 addition & 1 deletion src/coordinator/metrics_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl MetricsStore {
self.rx.borrow().get(key).cloned()
}

#[cfg(test)]
#[cfg(all(test, feature = "grpc"))]
pub(crate) fn from_entries(entries: impl IntoIterator<Item = (TaskKey, TaskMetrics)>) -> Self {
let map: HashMap<_, _> = entries.into_iter().collect();
let (tx, rx) = watch::channel(map);
Expand Down
2 changes: 1 addition & 1 deletion src/execution_plans/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl MetricsWrapperExec {
Self { inner, metrics }
}

#[cfg(test)]
#[cfg(all(test, feature = "grpc"))]
pub(crate) fn inner(&self) -> &Arc<dyn ExecutionPlan> {
&self.inner
}
Expand Down
4 changes: 3 additions & 1 deletion src/execution_plans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ mod network_coalesce;
mod network_shuffle;
mod sampler;

#[cfg(any(test, feature = "integration"))]
// The benchmark fixtures spin up real gRPC workers (`tonic` servers, Flight channels), so they
// only exist when that transport is compiled in.
#[cfg(all(feature = "grpc", any(test, feature = "integration")))]
pub mod benchmarks;

pub use broadcast::BroadcastExec;
Expand Down
13 changes: 9 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,19 @@ pub mod test_utils;
#[cfg(feature = "grpc")]
pub use protocol::grpc;

/// The worker-protocol prost message types, independent of any transport. A non-gRPC transport
/// reaches for these to speak the same wire shape the gRPC path serializes. Unstable: this
/// mirrors `worker.proto`, which is regenerated freely.
pub use protocol::generated::worker as proto;

pub use codec::DistributedCodec;
pub use worker_resolver::{WorkerResolver, get_distributed_worker_resolver};

pub use protocol::{
ChannelResolver, CoordinatorToWorkerMsg, ExecuteTaskRequest, GetWorkerInfoRequest,
GetWorkerInfoResponse, LoadInfo, ProducerHeadSpec, SetPlanRequest, TaskKey, TaskMetrics,
WorkUnitBatch, WorkUnitFeedDeclaration, WorkUnitMsg, WorkerChannel, WorkerToCoordinatorMsg,
get_distributed_channel_resolver,
GetWorkerInfoResponse, InProcessChannelResolver, LoadInfo, ProducerHeadSpec, SetPlanRequest,
TaskKey, TaskMetrics, WorkUnitBatch, WorkUnitFeedDeclaration, WorkUnitMsg, WorkerChannel,
WorkerToCoordinatorMsg, get_distributed_channel_resolver,
};
pub use stage::{
DistributedTaskContext, Stage, display_plan_ascii, display_plan_graphviz, explain_analyze,
Expand All @@ -59,7 +64,7 @@ pub use worker::{
Worker, WorkerQueryContext, WorkerSessionBuilder,
};

#[cfg(any(feature = "integration", test))]
#[cfg(all(feature = "grpc", any(feature = "integration", test)))]
pub use execution_plans::benchmarks::{
LocalRepartitionBench, LocalRepartitionFixture, LocalRepartitionMode, ShuffleBench,
ShuffleFixture, TransportBench, TransportBenchMode, TransportFixture,
Expand Down
3 changes: 2 additions & 1 deletion src/metrics/task_metrics_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ pub fn collect_plan_metrics(plan: &Arc<dyn ExecutionPlan>) -> Result<Vec<Metrics
Ok(metrics)
}

#[cfg(test)]
// These tests execute over the in-memory gRPC transport, so they need that transport compiled in.
#[cfg(all(test, feature = "grpc"))]
mod tests {

use super::*;
Expand Down
3 changes: 2 additions & 1 deletion src/metrics/task_metrics_rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ pub fn stage_metrics_rewriter(
.map(|v| v.data)
}

#[cfg(test)]
// These tests execute over the in-memory gRPC transport, so they need that transport compiled in.
#[cfg(all(test, feature = "grpc"))]
mod tests {
use crate::DistributedExt;
use crate::coordinator::MetricsStore;
Expand Down
3 changes: 3 additions & 0 deletions src/protocol/channel_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ pub fn get_distributed_channel_resolver(

#[cfg(not(feature = "grpc"))]
{
// With `grpc` off there is no built-in default. A co-located deployment can register
// [`crate::InProcessChannelResolver`] (or another transport) via
// `with_distributed_channel_resolver`.
panic!(
"gRPC feature is not enabled, and no channel resolver was provided, so no default ChannelResolver can be provided"
);
Expand Down
1 change: 1 addition & 0 deletions src/protocol/generated/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod worker;
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ pub struct MaxGauge {
pub value: u64,
}
/// Generated client implementations.
#[cfg(feature = "grpc")]
pub mod worker_service_client {
#![allow(
unused_variables,
Expand Down Expand Up @@ -617,6 +618,7 @@ pub mod worker_service_client {
}
}
/// Generated server implementations.
#[cfg(feature = "grpc")]
pub mod worker_service_server {
#![allow(
unused_variables,
Expand Down
6 changes: 5 additions & 1 deletion src/protocol/grpc/gen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {

let proto_dir = repo_root.join("src/protocol/grpc");
let proto_file = proto_dir.join("worker.proto");
let out_dir = repo_root.join("src/protocol/grpc/generated");
let out_dir = repo_root.join("src/protocol/generated");

fs::create_dir_all(&out_dir)?;

Expand All @@ -18,6 +18,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
tonic_prost_build::configure()
.build_server(true)
.build_client(true)
// The generated messages build with `grpc` off; only the tonic client and server carry
// the feature gate. Emitted here so a regeneration cannot drop the gates.
.client_mod_attribute(".", "#[cfg(feature = \"grpc\")]")
.server_mod_attribute(".", "#[cfg(feature = \"grpc\")]")
.out_dir(&out_dir)
.extern_path(".worker.FlightData", "::arrow_flight::FlightData")
.extern_path(
Expand Down
1 change: 0 additions & 1 deletion src/protocol/grpc/generated/mod.rs

This file was deleted.

2 changes: 1 addition & 1 deletion src/protocol/grpc/metrics_proto.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::generated::worker as pb;
use crate::protocol::generated::worker as pb;
use chrono::DateTime;
use datafusion::common::internal_err;
use datafusion::error::DataFusionError;
Expand Down
1 change: 0 additions & 1 deletion src/protocol/grpc/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
mod channel_resolver;
mod errors;
mod generated;
mod metrics_proto;
mod observability;
mod on_drop_stream;
Expand Down
2 changes: 1 addition & 1 deletion src/protocol/grpc/observability/gen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
.out_dir(&out_dir)
.extern_path(
".observability.TaskKey",
"crate::protocol::grpc::generated::worker::TaskKey",
"crate::protocol::generated::worker::TaskKey",
)
.compile_protos(&[proto_file], &[proto_dir])?;

Expand Down
Loading