Skip to content

Commit 3eaf677

Browse files
committed
fix(query): avoid serial distribution contagion in joins
1 parent db003ef commit 3eaf677

7 files changed

Lines changed: 452 additions & 26 deletions

File tree

src/query/service/src/schedulers/fragments/fragmenter.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,22 @@ impl Fragmenter {
116116

117117
let edges = Self::collect_fragments_edge(fragments.values());
118118

119-
for (source, target) in edges {
120-
let Some(fragment) = fragments.get_mut(&source) else {
119+
for (source, target) in &edges {
120+
let Some(fragment) = fragments.get_mut(source) else {
121121
continue;
122122
};
123123

124124
if let Some(exchange_sink) = ExchangeSink::from_mut_physical_plan(&mut fragment.plan) {
125-
exchange_sink.destination_fragment_id = target;
125+
exchange_sink.destination_fragment_id = *target;
126+
}
127+
}
128+
129+
for (source, target) in edges {
130+
let Some(source_fragment) = fragments.get(&source).cloned() else {
131+
continue;
132+
};
133+
if let Some(target_fragment) = fragments.get_mut(&target) {
134+
target_fragment.source_fragments.push(source_fragment);
126135
}
127136
}
128137

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use databend_common_expression::DataSchemaRefExt;
16+
use databend_common_sql::executor::physical_plans::FragmentKind;
17+
use databend_query::physical_plans::ConstantTableScan;
18+
use databend_query::physical_plans::Exchange;
19+
use databend_query::physical_plans::PhysicalPlan;
20+
use databend_query::physical_plans::PhysicalPlanMeta;
21+
use databend_query::schedulers::Fragmenter;
22+
use databend_query::schedulers::QueryFragmentsActions;
23+
use databend_query::servers::flight::v1::exchange::DataExchange;
24+
use databend_query::test_kits::ClusterDescriptor;
25+
use databend_query::test_kits::TestFixture;
26+
27+
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
28+
async fn test_merge_dependent_fragment_runs_on_coordinator() -> anyhow::Result<()> {
29+
let fixture = TestFixture::setup().await?;
30+
let cluster = ClusterDescriptor::new()
31+
.with_node("coordinator", "127.0.0.1:19001")
32+
.with_node("worker", "127.0.0.1:19002")
33+
.with_local_id("coordinator");
34+
let ctx = fixture.new_query_ctx_with_cluster(cluster).await?;
35+
36+
let scan = PhysicalPlan::new(ConstantTableScan {
37+
values: vec![],
38+
num_rows: 1,
39+
output_schema: DataSchemaRefExt::create(vec![]),
40+
meta: PhysicalPlanMeta::new("ConstantTableScan"),
41+
});
42+
let merge = PhysicalPlan::new(Exchange {
43+
input: scan,
44+
kind: FragmentKind::Merge,
45+
keys: vec![],
46+
ignore_exchange: false,
47+
allow_adjust_parallelism: true,
48+
meta: PhysicalPlanMeta::new("Exchange"),
49+
});
50+
let broadcast = PhysicalPlan::new(Exchange {
51+
input: merge,
52+
kind: FragmentKind::Expansive,
53+
keys: vec![],
54+
ignore_exchange: false,
55+
allow_adjust_parallelism: true,
56+
meta: PhysicalPlanMeta::new("Exchange"),
57+
});
58+
59+
let fragments = Fragmenter::try_create(ctx.clone())?.build_fragment(&broadcast)?;
60+
let broadcast_fragment = fragments
61+
.iter()
62+
.find(|fragment| matches!(&fragment.exchange, Some(DataExchange::Broadcast(_))))
63+
.expect("broadcast fragment");
64+
let mut actions = QueryFragmentsActions::create(ctx.clone());
65+
broadcast_fragment.get_actions(ctx.clone(), &mut actions)?;
66+
67+
let broadcast_actions = actions
68+
.fragments_actions
69+
.first()
70+
.expect("broadcast fragment actions");
71+
assert_eq!(broadcast_actions.fragment_actions.len(), 1);
72+
assert_eq!(
73+
broadcast_actions.fragment_actions[0].executor,
74+
"coordinator"
75+
);
76+
77+
Ok(())
78+
}

src/query/service/tests/it/distributed/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@
1313
// limitations under the License.
1414

1515
mod cluster;
16+
mod fragmenter;

src/query/sql/src/planner/plans/join.rs

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,27 @@ impl Join {
746746
&& !ctx.get_cluster().is_empty()
747747
&& self.spatial_join_candidate(rel_expr)?.is_some())
748748
}
749+
750+
fn broadcast_build_is_preferred(ctx: &dyn TableContext, rel_expr: &RelExpr) -> Result<bool> {
751+
let settings = ctx.get_settings();
752+
if ctx.get_cluster().is_empty() || settings.get_enforce_shuffle_join()? {
753+
return Ok(false);
754+
}
755+
756+
let left_cardinality = rel_expr.derive_cardinality_child(0)?.cardinality;
757+
let right_cardinality = rel_expr.derive_cardinality_child(1)?.cardinality;
758+
let broadcast_join_threshold = if settings.get_prefer_broadcast_join()? {
759+
(ctx.get_cluster().nodes.len() - 1) as f64
760+
} else {
761+
// Use a very large value to prevent broadcast join.
762+
1000.0
763+
};
764+
765+
Ok(
766+
right_cardinality * broadcast_join_threshold < left_cardinality
767+
|| settings.get_enforce_broadcast_join()?,
768+
)
769+
}
749770
}
750771

751772
impl Operator for Join {
@@ -901,7 +922,40 @@ impl Operator for Join {
901922
return Ok(required);
902923
}
903924

904-
// if join/probe side is Serial or this is a non-equi join, we use Serial distribution
925+
// A Serial build can still be redistributed. Broadcasting a small build
926+
// keeps the probe distributed instead of propagating Serial to both sides.
927+
let has_only_non_equi_conditions =
928+
self.equi_conditions.is_empty() && !self.non_equi_conditions.is_empty();
929+
if build_physical_prop.distribution == Distribution::Serial
930+
&& probe_physical_prop.distribution != Distribution::Serial
931+
&& !has_only_non_equi_conditions
932+
&& !matches!(
933+
self.join_type,
934+
JoinType::Right
935+
| JoinType::Full
936+
| JoinType::RightAnti
937+
| JoinType::RightSemi
938+
| JoinType::LeftMark
939+
| JoinType::RightSingle
940+
| JoinType::InnerAny
941+
| JoinType::LeftAny
942+
| JoinType::RightAny
943+
| JoinType::Asof
944+
| JoinType::LeftAsof
945+
| JoinType::RightAsof
946+
| JoinType::FullAsof
947+
)
948+
&& Self::broadcast_build_is_preferred(ctx.as_ref(), rel_expr)?
949+
{
950+
required.distribution = if child_index == 1 {
951+
Distribution::Broadcast
952+
} else {
953+
Distribution::Any
954+
};
955+
return Ok(required);
956+
}
957+
958+
// If either side remains Serial or this is a non-equi join, use Serial distribution.
905959
if probe_physical_prop.distribution == Distribution::Serial
906960
|| build_physical_prop.distribution == Distribution::Serial
907961
|| (self.equi_conditions.is_empty() && !self.non_equi_conditions.is_empty())
@@ -912,7 +966,6 @@ impl Operator for Join {
912966
}
913967

914968
// Try to use broadcast join
915-
let settings = ctx.get_settings();
916969
if !matches!(
917970
self.join_type,
918971
JoinType::Right
@@ -927,29 +980,14 @@ impl Operator for Join {
927980
| JoinType::LeftAsof
928981
| JoinType::RightAsof
929982
| JoinType::FullAsof
930-
) {
931-
let left_stat_info = rel_expr.derive_cardinality_child(0)?;
932-
let right_stat_info = rel_expr.derive_cardinality_child(1)?;
933-
// The broadcast join is cheaper than the hash join when one input is at least (n − 1)× larger than the other
934-
// where n is the number of servers in the cluster.
935-
let broadcast_join_threshold = if settings.get_prefer_broadcast_join()? {
936-
(ctx.get_cluster().nodes.len() - 1) as f64
983+
) && Self::broadcast_build_is_preferred(ctx.as_ref(), rel_expr)?
984+
{
985+
required.distribution = if child_index == 1 {
986+
Distribution::Broadcast
937987
} else {
938-
// Use a very large value to prevent broadcast join.
939-
1000.0
988+
Distribution::Any
940989
};
941-
if !settings.get_enforce_shuffle_join()?
942-
&& (right_stat_info.cardinality * broadcast_join_threshold
943-
< left_stat_info.cardinality
944-
|| settings.get_enforce_broadcast_join()?)
945-
{
946-
if child_index == 1 {
947-
required.distribution = Distribution::Broadcast;
948-
} else {
949-
required.distribution = Distribution::Any;
950-
}
951-
return Ok(required);
952-
}
990+
return Ok(required);
953991
}
954992

955993
// Otherwise, use hash shuffle
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::collections::HashMap;
16+
17+
use databend_common_catalog::table_context::TableContextSettings;
18+
use databend_common_exception::Result;
19+
20+
use super::table_statistics;
21+
use crate::framework::LiteTableContext;
22+
use crate::framework::golden::SqlTestCase;
23+
use crate::framework::golden::open_golden_file;
24+
use crate::framework::golden::write_case_header;
25+
26+
async fn write_distributed_case(
27+
file: &mut impl std::io::Write,
28+
case: &SqlTestCase,
29+
probe_rows: u64,
30+
build_rows: u64,
31+
) -> Result<()> {
32+
let ctx = LiteTableContext::create().await?;
33+
ctx.set_cluster_node_num(2);
34+
ctx.set_table_warehouse_distribution(true);
35+
let settings = ctx.get_settings();
36+
settings.set_setting("disable_join_reorder".to_string(), "1".to_string())?;
37+
ctx.register_table_sql_with_stats(
38+
BIG_TABLE,
39+
Some(table_statistics(probe_rows)),
40+
HashMap::new(),
41+
HashMap::new(),
42+
)
43+
.await?;
44+
ctx.register_table_sql_with_stats(
45+
SMALL_TABLE,
46+
Some(table_statistics(build_rows)),
47+
HashMap::new(),
48+
HashMap::new(),
49+
)
50+
.await?;
51+
52+
let raw_plan = ctx.bind_sql(case.sql).await?;
53+
let optimized_plan = ctx.optimize_plan(raw_plan.clone()).await?;
54+
55+
write_case_header(file, case)?;
56+
writeln!(file, "raw_plan:")?;
57+
writeln!(file, "{}", raw_plan.format_indent(Default::default())?)?;
58+
writeln!(file, "optimized_plan:")?;
59+
writeln!(
60+
file,
61+
"{}",
62+
optimized_plan.format_indent(Default::default())?
63+
)?;
64+
writeln!(file)?;
65+
66+
Ok(())
67+
}
68+
69+
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
70+
async fn test_serial_build_distribution() -> Result<()> {
71+
let mut file = open_golden_file("optimizer", "distributed_join.txt")?;
72+
let cases = [
73+
(
74+
SqlTestCase {
75+
name: "small_serial_build_is_broadcast",
76+
description: "A small scalar aggregate on the build side should be broadcast without merging the distributed probe.",
77+
setup_sqls: &[],
78+
sql: "SELECT b.k, s.max_k
79+
FROM big_table AS b
80+
LEFT JOIN (SELECT max(k) AS max_k FROM small_table) AS s
81+
ON b.k = s.max_k",
82+
},
83+
227_000_000,
84+
24,
85+
),
86+
(
87+
SqlTestCase {
88+
name: "cross_join_small_serial_build_is_broadcast",
89+
description: "A small scalar aggregate should also be broadcast for a cross join without merging the distributed probe.",
90+
setup_sqls: &[],
91+
sql: "SELECT b.k, s.max_k
92+
FROM big_table AS b
93+
CROSS JOIN (SELECT max(k) AS max_k FROM small_table) AS s",
94+
},
95+
227_000_000,
96+
24,
97+
),
98+
(
99+
SqlTestCase {
100+
name: "large_serial_build_remains_serial",
101+
description: "A Serial build that is not smaller than the probe should not be broadcast unconditionally.",
102+
setup_sqls: &[],
103+
sql: "SELECT b.k, s.k
104+
FROM big_table AS b
105+
LEFT JOIN (
106+
SELECT k, row_number() OVER () AS row_num
107+
FROM small_table
108+
) AS s
109+
ON b.k = s.k",
110+
},
111+
1_000,
112+
1_000,
113+
),
114+
];
115+
116+
for (case, probe_rows, build_rows) in &cases {
117+
write_distributed_case(&mut file, case, *probe_rows, *build_rows).await?;
118+
}
119+
120+
Ok(())
121+
}
122+
123+
const BIG_TABLE: &str = "CREATE TABLE big_table (k BIGINT)";
124+
const SMALL_TABLE: &str = "CREATE TABLE small_table (k BIGINT)";

0 commit comments

Comments
 (0)