Skip to content

Commit 336d138

Browse files
antiguruclaude
andcommitted
compute: Retire the fueled as_specific_collection flag
`as_specific_collection`'s keyed path was gated on `ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION` (default true): the fueled branch packs the arrangement cursor into a `ColumnBuilder` and returns the columnar edge, while the off branch called the deprecated `ArrangementFlavor::as_collection` and returned a real-data `Vec`. The off branch was the last non-leaf real-data `Vec` producer. The fueled path is a complete functional replacement: it materializes every `(row, t, diff)` from the same cursor with all columns decoded, so production (flag on) already used it. Delete the off branch, always take the fueled columnar path, and retire the flag from `dyncfgs.rs`, its config-set registration, and the LaunchDarkly and parallel-workload flag lists. With the flag gone, `ArrangementFlavor::as_collection` has no callers, so remove it. The `config_set` argument then existed only to reach the flag, threaded through `as_specific_collection` and `as_collection_core` (whose sole `config_set` use was forwarding it), so drop it from both signatures and their callers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 083529c commit 336d138

9 files changed

Lines changed: 36 additions & 113 deletions

File tree

misc/python/materialize/mzcompose/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,6 @@ def get_default_system_parameters(
588588
"compute_mv_sink_advance_persist_frontiers",
589589
"compute_prometheus_introspection_scrape_interval",
590590
"enable_compute_replica_expiration",
591-
"enable_compute_render_fueled_as_specific_collection",
592591
"compute_logical_backpressure_max_retained_capabilities",
593592
"compute_logical_backpressure_inflight_slack",
594593
"persist_fetch_semaphore_cost_adjustment",

misc/python/materialize/parallel_workload/action.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1911,7 +1911,6 @@ def __init__(
19111911
"enable_compute_replica_expiration",
19121912
"compute_mv_sink_advance_persist_frontiers",
19131913
"compute_replica_expiration_offset",
1914-
"enable_compute_render_fueled_as_specific_collection",
19151914
"compute_temporal_bucketing_summary",
19161915
"enable_compute_logical_backpressure",
19171916
"enable_replica_targeted_materialized_views",

src/compute-types/src/dyncfgs.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,6 @@ pub const COMPUTE_FLAT_MAP_FUEL: Config<usize> = Config::new(
346346
"The amount of output the flat-map operator produces before yielding.",
347347
);
348348

349-
/// Whether to render `as_specific_collection` using a fueled flat-map operator.
350-
pub const ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION: Config<bool> = Config::new(
351-
"enable_compute_render_fueled_as_specific_collection",
352-
true,
353-
"When enabled, renders `as_specific_collection` using a fueled flat-map operator.",
354-
);
355-
356349
/// Whether to apply logical backpressure in compute dataflows.
357350
pub const ENABLE_COMPUTE_LOGICAL_BACKPRESSURE: Config<bool> = Config::new(
358351
"enable_compute_logical_backpressure",
@@ -520,7 +513,6 @@ pub fn all_dyncfgs(configs: ConfigSet) -> ConfigSet {
520513
.add(&COMPUTE_APPLY_COLUMN_DEMANDS)
521514
.add(&COMPUTE_FLAT_MAP_FUEL)
522515
.add(&CONSOLIDATING_VEC_GROWTH_DAMPENER)
523-
.add(&ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION)
524516
.add(&ENABLE_COMPUTE_LOGICAL_BACKPRESSURE)
525517
.add(&COMPUTE_LOGICAL_BACKPRESSURE_MAX_RETAINED_CAPABILITIES)
526518
.add(&COMPUTE_LOGICAL_BACKPRESSURE_INFLIGHT_SLACK)

src/compute/src/render.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,17 +1259,12 @@ impl<'scope, T: RenderTimestamp + MaybeBucketByTime> Context<'scope, T> {
12591259
mfp,
12601260
Some((key, row)),
12611261
self.until.clone(),
1262-
&self.config_set,
12631262
);
12641263
CollectionBundle::from_edge(oks, errs)
12651264
}
12661265
mz_compute_types::plan::GetPlan::Collection(mfp) => {
1267-
let (oks, errs) = collection.as_collection_core(
1268-
mfp,
1269-
None,
1270-
self.until.clone(),
1271-
&self.config_set,
1272-
);
1266+
let (oks, errs) =
1267+
collection.as_collection_core(mfp, None, self.until.clone());
12731268
CollectionBundle::from_edge(oks, errs)
12741269
}
12751270
}
@@ -1284,12 +1279,8 @@ impl<'scope, T: RenderTimestamp + MaybeBucketByTime> Context<'scope, T> {
12841279
if mfp.is_identity() {
12851280
input
12861281
} else {
1287-
let (oks, errs) = input.as_collection_core(
1288-
mfp,
1289-
input_key_val,
1290-
self.until.clone(),
1291-
&self.config_set,
1292-
);
1282+
let (oks, errs) =
1283+
input.as_collection_core(mfp, input_key_val, self.until.clone());
12931284
CollectionBundle::from_edge(oks, errs)
12941285
}
12951286
}

src/compute/src/render/context.rs

Lines changed: 26 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ use differential_dataflow::trace::{Cursor, Navigable, TraceReader};
2222
use differential_dataflow::{AsCollection, VecCollection};
2323
use mz_compute_types::dataflows::DataflowDescription;
2424
use mz_compute_types::dyncfgs::{
25-
ENABLE_COLUMN_PAGED_BATCHER, ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION,
26-
ENABLE_COMPUTE_TEMPORAL_BUCKETING, TEMPORAL_BUCKETING_SUMMARY,
25+
ENABLE_COLUMN_PAGED_BATCHER, ENABLE_COMPUTE_TEMPORAL_BUCKETING, TEMPORAL_BUCKETING_SUMMARY,
2726
};
2827
use mz_compute_types::plan::scalar::{LirScalarExpr, mfp_mir_to_lir_plan, mfp_plan_lir_to_mir};
2928
use mz_compute_types::plan::{ArrangementStrategy, AvailableCollections};
@@ -57,7 +56,7 @@ use crate::render::{LinearJoinSpec, MaybeBucketByTime, RenderTimestamp};
5756
use crate::typedefs::{
5857
ErrAgent, ErrBatcher, ErrBuilder, ErrEnter, ErrSpine, RowRowAgent, RowRowEnter, RowRowSpine,
5958
};
60-
use mz_row_spine::{DatumSeq, RowRowBuilder, RowRowColPagedBuilder};
59+
use mz_row_spine::{RowRowBuilder, RowRowColPagedBuilder};
6160

6261
/// Dataflow-local collections and arrangements.
6362
///
@@ -236,40 +235,6 @@ pub enum ArrangementFlavor<'scope, T: RenderTimestamp> {
236235
}
237236

238237
impl<'scope, T: RenderTimestamp> ArrangementFlavor<'scope, T> {
239-
/// Presents `self` as a stream of updates.
240-
///
241-
/// Deprecated: This function is not fueled and hence risks flattening the whole arrangement.
242-
///
243-
/// This method presents the contents as they are, without further computation.
244-
/// If you have logic that could be applied to each record, consider using the
245-
/// `flat_map` methods which allows this and can reduce the work done.
246-
#[deprecated(note = "Use `flat_map` instead.")]
247-
pub fn as_collection(
248-
&self,
249-
) -> (
250-
VecCollection<'scope, T, Row, Diff>,
251-
VecCollection<'scope, T, DataflowErrorSer, Diff>,
252-
) {
253-
let mut datums = DatumVec::new();
254-
let logic = move |k: DatumSeq, v: DatumSeq| {
255-
let temp_storage = RowArena::new();
256-
let mut datums_borrow = datums.borrow();
257-
k.extend_datums(&temp_storage, &mut datums_borrow, None);
258-
v.extend_datums(&temp_storage, &mut datums_borrow, None);
259-
SharedRow::pack(&**datums_borrow)
260-
};
261-
match &self {
262-
ArrangementFlavor::Local(oks, errs) => (
263-
oks.clone().as_collection(logic),
264-
errs.clone().as_collection(|k, &()| k.clone()),
265-
),
266-
ArrangementFlavor::Trace(_, oks, errs) => (
267-
oks.clone().as_collection(logic),
268-
errs.clone().as_collection(|k, &()| k.clone()),
269-
),
270-
}
271-
}
272-
273238
/// Constructs and applies logic to elements of `self` and returns the results.
274239
///
275240
/// The `logic` callback receives a borrow of the decoded datum vector, a timestamp, a
@@ -558,9 +523,8 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
558523
/// Therefore, it should be used when the appropriate transformation
559524
/// was planned as part of a following MFP.
560525
///
561-
/// If `key` is specified, the function converts the arrangement to a collection. It uses either
562-
/// the fueled `flat_map` or `as_collection` method, depending on the flag
563-
/// [`ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION`].
526+
/// If `key` is specified, the function converts the arrangement to a collection using a
527+
/// fueled `flat_map` operator.
564528
///
565529
/// The keyed path materializes the arrangement as the columnar edge, so an
566530
/// arrangement-producing operator (Reduce, Threshold, bucketed TopK) whose
@@ -570,7 +534,6 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
570534
pub fn as_specific_collection(
571535
&self,
572536
key: Option<&[LirScalarExpr]>,
573-
config_set: &ConfigSet,
574537
) -> (
575538
CollectionEdge<'scope, T>,
576539
VecCollection<'scope, T, DataflowErrorSer, Diff>,
@@ -589,28 +552,22 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
589552
let arranged = self.arranged.get(key).unwrap_or_else(|| {
590553
panic!("The collection arranged by {:?} doesn't exist.", key)
591554
});
592-
if ENABLE_COMPUTE_RENDER_FUELED_AS_SPECIFIC_COLLECTION.get(config_set) {
593-
// Decode all columns (max_demand usize::MAX) and pack each cursor record
594-
// into a `Column`, so the materialized collection carries the columnar edge.
595-
// Output is 1:1 from the already-consolidated cursor, so a non-consolidating
596-
// `ColumnBuilder` matches the row-based `CapacityContainerBuilder` this
597-
// replaced; the packed row is pushed borrowed, holding no owned `Row` per
598-
// record.
599-
let (ok, err) = arranged.flat_map_ok::<ColumnBuilder<(Row, T, Diff)>, _>(
600-
None,
601-
usize::MAX,
602-
|borrow, t, r, ok_session| {
603-
let row = SharedRow::pack(borrow.iter());
604-
ok_session.give((&row, &t, &r));
605-
1
606-
},
607-
);
608-
(CollectionEdge::Columnar(ok.as_collection()), err)
609-
} else {
610-
#[allow(deprecated)]
611-
let (oks, errs) = arranged.as_collection();
612-
(CollectionEdge::Vec(oks), errs)
613-
}
555+
// Decode all columns (max_demand usize::MAX) and pack each cursor record
556+
// into a `Column`, so the materialized collection carries the columnar edge.
557+
// Output is 1:1 from the already-consolidated cursor, so a non-consolidating
558+
// `ColumnBuilder` matches the row-based `CapacityContainerBuilder` this
559+
// replaced; the packed row is pushed borrowed, holding no owned `Row` per
560+
// record.
561+
let (ok, err) = arranged.flat_map_ok::<ColumnBuilder<(Row, T, Diff)>, _>(
562+
None,
563+
usize::MAX,
564+
|borrow, t, r, ok_session| {
565+
let row = SharedRow::pack(borrow.iter());
566+
ok_session.give((&row, &t, &r));
567+
1
568+
},
569+
);
570+
(CollectionEdge::Columnar(ok.as_collection()), err)
614571
}
615572
}
616573
}
@@ -921,7 +878,6 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
921878
mfp_plan: MfpPlan<LirScalarExpr>,
922879
key_val: Option<(Vec<LirScalarExpr>, Option<Row>)>,
923880
until: Antichain<mz_repr::Timestamp>,
924-
config_set: &ConfigSet,
925881
) -> (
926882
CollectionEdge<'scope, T>,
927883
VecCollection<'scope, T, DataflowErrorSer, Diff>,
@@ -949,7 +905,7 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
949905
// Keyed identity materializes an existing arrangement.
950906
// `as_specific_collection` returns the columnar edge, so the
951907
// reduce/threshold/topk result carries columnar downstream.
952-
Some(key) => self.as_specific_collection(Some(&key), config_set),
908+
Some(key) => self.as_specific_collection(Some(&key)),
953909
};
954910
}
955911

@@ -1066,7 +1022,7 @@ impl<'scope, T: RenderTimestamp> CollectionBundle<'scope, T> {
10661022
let form_raw_collection = collections.raw || will_create_arrangement;
10671023
if form_raw_collection && self.collection.is_none() {
10681024
let (oks, errs) =
1069-
self.as_collection_core(input_mfp, input_key.map(|k| (k, None)), until, config_set);
1025+
self.as_collection_core(input_mfp, input_key.map(|k| (k, None)), until);
10701026
// Apply temporal bucketing when the lowering selected `TemporalBucketing` and
10711027
// we will build at least one arrangement. This path fires when the collection
10721028
// must be formed from scratch (e.g., from an arrangement via as_collection_core).
@@ -1523,7 +1479,6 @@ fn walk_cursor<C, F>(
15231479
#[cfg(test)]
15241480
mod tests {
15251481
use differential_dataflow::input::Input;
1526-
use mz_compute_types::dyncfgs::all_dyncfgs;
15271482
use mz_expr::{EvalError, MapFilterProject};
15281483
use mz_repr::{Datum, ReprScalarType, Timestamp};
15291484
use timely::dataflow::operators::Capture;
@@ -1727,15 +1682,13 @@ mod tests {
17271682
.collect();
17281683
expected.sort();
17291684

1730-
let config_set = ConfigSet::default();
17311685
let (producer_is_columnar, passthrough_is_columnar, produced) =
17321686
timely::execute_directly(move |worker| {
17331687
worker.dataflow::<Timestamp, _, _>(|scope| {
17341688
let (mut input, collection) = scope.new_collection();
17351689
let (_err_input, errs) = scope.new_collection::<DataflowErrorSer, Diff>();
17361690
let bundle = CollectionBundle::from_edge(CollectionEdge::Vec(collection), errs);
1737-
let (edge, _errs) =
1738-
bundle.as_collection_core(mfp, None, Antichain::new(), &config_set);
1691+
let (edge, _errs) = bundle.as_collection_core(mfp, None, Antichain::new());
17391692
let producer_is_columnar = matches!(edge, CollectionEdge::Columnar(_));
17401693
// Tee the producer output for a content check, then feed the
17411694
// original edge into the arrange input.
@@ -1778,7 +1731,6 @@ mod tests {
17781731
#[mz_ore::test]
17791732
fn as_collection_core_identity_passes_edge_through() {
17801733
for columnar_input in [false, true] {
1781-
let config_set = ConfigSet::default();
17821734
let is_columnar = timely::execute_directly(move |worker| {
17831735
worker.dataflow::<Timestamp, _, _>(|scope| {
17841736
let (mut input, collection) = scope.new_collection::<Row, Diff>();
@@ -1792,8 +1744,7 @@ mod tests {
17921744
let identity = MapFilterProject::<LirScalarExpr>::new(1)
17931745
.into_plan()
17941746
.expect("identity mfp");
1795-
let (out, _errs) =
1796-
bundle.as_collection_core(identity, None, Antichain::new(), &config_set);
1747+
let (out, _errs) = bundle.as_collection_core(identity, None, Antichain::new());
17971748
let is_columnar = matches!(out, CollectionEdge::Columnar(_));
17981749
input.update(Row::pack_slice(&[Datum::Int64(1)]), Diff::ONE);
17991750
input.advance_to(Timestamp::from(1u64));
@@ -1839,14 +1790,12 @@ mod tests {
18391790
),
18401791
];
18411792

1842-
let config_set = ConfigSet::default();
18431793
let captured = timely::execute_directly(move |worker| {
18441794
worker.dataflow::<Timestamp, _, _>(|scope| {
18451795
let (mut input, collection) = scope.new_collection();
18461796
let (_err_input, errs) = scope.new_collection::<DataflowErrorSer, Diff>();
18471797
let bundle = CollectionBundle::from_edge(CollectionEdge::Vec(collection), errs);
1848-
let (edge, _errs) =
1849-
bundle.as_collection_core(mfp, None, Antichain::new(), &config_set);
1798+
let (edge, _errs) = bundle.as_collection_core(mfp, None, Antichain::new());
18501799
assert!(
18511800
matches!(edge, CollectionEdge::Columnar(_)),
18521801
"a non-identity MFP must produce a columnar edge"
@@ -1887,9 +1836,6 @@ mod tests {
18871836
.collect();
18881837
expected.sort();
18891838

1890-
// A populated set so the fueled-materialization flag resolves to its
1891-
// default (`true`); `ConfigSet::default()` alone would panic on lookup.
1892-
let config_set = all_dyncfgs(ConfigSet::default());
18931839
let (is_columnar, captured) = timely::execute_directly(move |worker| {
18941840
worker.dataflow::<Timestamp, _, _>(|scope| {
18951841
let (mut input, collection) = scope.new_collection();
@@ -1915,7 +1861,7 @@ mod tests {
19151861
0..1,
19161862
ArrangementFlavor::Local(arranged, err_arranged),
19171863
);
1918-
let (edge, _errs) = bundle.as_specific_collection(Some(&key), &config_set);
1864+
let (edge, _errs) = bundle.as_specific_collection(Some(&key));
19191865
let is_columnar = matches!(edge, CollectionEdge::Columnar(_));
19201866
let captured = edge.into_vec().inner.capture();
19211867

src/compute/src/render/flat_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ impl<'scope, T: crate::render::RenderTimestamp> Context<'scope, T> {
6161
.collection
6262
.clone()
6363
.expect("The unarranged collection doesn't exist."),
64-
Some(key) => input.as_specific_collection(Some(key), &self.config_set),
64+
Some(key) => input.as_specific_collection(Some(key)),
6565
};
6666

6767
let (oks, errs) = match edge {

src/compute/src/render/join/linear_join.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,9 @@ where
266266
// columnar edge. `differential_join` forms the source key off
267267
// this edge below, so the columnar source has no `ColumnarToVec`
268268
// hop.
269-
Some(key) => inputs[linear_plan.source_relation]
270-
.as_specific_collection(Some(key), &self.config_set),
269+
Some(key) => {
270+
inputs[linear_plan.source_relation].as_specific_collection(Some(key))
271+
}
271272
};
272273
errors.push(errs.enter_region(inner));
273274
let joined = joined.enter_region(inner);

src/compute/src/render/sinks.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,8 @@ impl<'g, T: RenderTimestamp> Context<'g, T> {
8383
// The sink serializes rows, so decode to `Vec` here. This is the
8484
// sanctioned sink leaf, the same seam as the raw-collection arm
8585
// above.
86-
let (oks, errs) = bundle.as_collection_core(
87-
mfp_plan,
88-
Some((key.clone(), None)),
89-
self.until.clone(),
90-
&self.config_set,
91-
);
86+
let (oks, errs) =
87+
bundle.as_collection_core(mfp_plan, Some((key.clone(), None)), self.until.clone());
9288
(oks.into_vec(), errs)
9389
};
9490

test/launchdarkly-flag-consistency/mzcompose.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@
244244
enable_coalesce_case_transform
245245
enable_cluster_controller
246246
enable_compute_half_join2
247-
enable_compute_render_fueled_as_specific_collection
248247
enable_date_bin_hopping
249248
enable_default_connection_validation
250249
enable_dequadratic_eqprop_map

0 commit comments

Comments
 (0)