diff --git a/crates/adapters/src/static_compile/seroutput.rs b/crates/adapters/src/static_compile/seroutput.rs index 052ad66ba82..2fca3090e54 100644 --- a/crates/adapters/src/static_compile/seroutput.rs +++ b/crates/adapters/src/static_compile/seroutput.rs @@ -541,7 +541,10 @@ where } fn into_trace(self: Arc) -> Box { - let mut spine = TypedBatch::new(DynSpine::::new(&B::factories())); + let mut spine = TypedBatch::new(DynSpine::::new( + &B::factories(), + Arc::new(String::from("SerTrace")), + )); TOKIO.block_on(spine.insert(Arc::unwrap_or_clone(self).batch.into_inner())); Box::new(SerBatchImpl::, KD, VD>::new(spine)) } diff --git a/crates/dbsp/Cargo.toml b/crates/dbsp/Cargo.toml index 74450c1f1bd..76785dab65d 100644 --- a/crates/dbsp/Cargo.toml +++ b/crates/dbsp/Cargo.toml @@ -46,11 +46,7 @@ arc-swap = { workspace = true } mimalloc-rust-sys = { workspace = true } rand = { workspace = true } rkyv = { workspace = true, features = ["std", "size_64", "validation", "uuid"] } -size-of = { workspace = true, features = [ - "hashbrown", - "xxhash-xxh3", - "arcstr" -] } +size-of = { workspace = true, features = [ "hashbrown", "xxhash-xxh3" ] } tokio-util = { workspace = true } futures = { workspace = true } tokio = { workspace = true, features = ["macros", "rt", "rt-multi-thread"] } diff --git a/crates/dbsp/src/circuit/circuit_builder.rs b/crates/dbsp/src/circuit/circuit_builder.rs index 83b50eae080..d4dc2c6012c 100644 --- a/crates/dbsp/src/circuit/circuit_builder.rs +++ b/crates/dbsp/src/circuit/circuit_builder.rs @@ -1196,7 +1196,7 @@ impl NodeId { self.0 } - pub(super) fn root() -> Self { + pub fn root() -> Self { Self(0) } } diff --git a/crates/dbsp/src/circuit/operator_traits.rs b/crates/dbsp/src/circuit/operator_traits.rs index da57e96ef9e..492b142a211 100644 --- a/crates/dbsp/src/circuit/operator_traits.rs +++ b/crates/dbsp/src/circuit/operator_traits.rs @@ -5,6 +5,7 @@ #![allow(async_fn_in_trait)] +use arc_swap::ArcSwap; use feldera_storage::{FileCommitter, StoragePath}; use crate::Error; @@ -16,6 +17,7 @@ use crate::{ trace::cursor::Position, }; use std::borrow::Cow; +use std::fmt::Display; use std::sync::Arc; use super::GlobalNodeId; @@ -628,3 +630,73 @@ pub trait ImportOperator: Operator { OwnershipPreference::INDIFFERENT } } + +/// The name of an operator, primarily for use in CPU profiles. +/// +/// An operator doesn't initially know its global node ID, but the global node +/// ID is useful for debugging and profiling. This type allows the name to +/// initially omit the ID but adds it when it becomes available. +pub struct OperatorName(ArcSwap); + +impl OperatorName { + /// Creates a new `OperatorName` that doesn't initially know the global node + /// ID, since an operator doesn't know this until [Operator::init] is + /// called. + /// + /// If the name is fetched, with [OperatorName::get], before + /// [OperatorName::init], then it will just returned as simply `(base)`. + pub fn new(base: &str) -> Self { + Self(ArcSwap::new(Arc::new(format!("({base})")))) + } + + /// Updates the name to include `global_id`. This should normally be called + /// once, from the implementation of [Operator::init]. If it is called more + /// than once then later calls are ignored. + /// + /// After this function is called, the name will have the format `base + /// nodeid` where `nodeid` is in the format used by the circuit profiler so + /// that it's easy to find by searching. + pub fn init(&self, global_id: &GlobalNodeId) { + let s = self.0.load(); + if let Some(rest) = s.strip_prefix('(') + && let Some(base) = rest.strip_suffix(')') + { + self.0 + .store(Arc::new(format!("{base} {}", global_id.node_identifier()))); + } + } + + /// Returns a copy of the operator name string. + pub fn get(&self) -> Arc { + self.0.load_full() + } +} + +impl Display for OperatorName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0.load()) + } +} + +#[cfg(test)] +mod tests { + use crate::circuit::{GlobalNodeId, NodeId, operator_traits::OperatorName}; + + #[test] + fn operator_name() { + let name = OperatorName::new("Name"); + assert_eq!(name.to_string(), "(Name)"); + name.init(&GlobalNodeId::from_path(&[ + NodeId::new(1), + NodeId::new(2), + NodeId::new(3), + ])); + assert_eq!(name.to_string(), "Name nn1_n2_n3"); + name.init(&GlobalNodeId::from_path(&[ + NodeId::new(4), + NodeId::new(5), + NodeId::new(6), + ])); + assert_eq!(name.to_string(), "Name nn1_n2_n3"); + } +} diff --git a/crates/dbsp/src/operator.rs b/crates/dbsp/src/operator.rs index 3a15a33b6bb..dc7f013f15c 100644 --- a/crates/dbsp/src/operator.rs +++ b/crates/dbsp/src/operator.rs @@ -58,8 +58,9 @@ pub mod star_join_macros; pub mod time_series; mod trace; +use std::fmt::Display; + use crate::Error; -use crate::circuit::GlobalNodeId; use crate::storage::backend::StorageError; pub use self::csv::CsvSource; @@ -96,9 +97,6 @@ pub use transaction_z1::TransactionZ1; pub use z1::{DelayedFeedback, DelayedNestedFeedback, Z1, Z1Nested}; /// Returns a `NoPersistentId` error if `persistent_id` is `None`. -fn require_persistent_id<'a>( - persistent_id: Option<&'a str>, - global_id: &GlobalNodeId, -) -> Result<&'a str, Error> { - persistent_id.ok_or_else(|| Error::Storage(StorageError::NoPersistentId(global_id.to_string()))) +fn require_persistent_id(persistent_id: Option<&str>, id: impl Display) -> Result<&str, Error> { + persistent_id.ok_or_else(|| Error::Storage(StorageError::NoPersistentId(id.to_string()))) } diff --git a/crates/dbsp/src/operator/communication/exchange.rs b/crates/dbsp/src/operator/communication/exchange.rs index 5350b0fe05d..29af8d87170 100644 --- a/crates/dbsp/src/operator/communication/exchange.rs +++ b/crates/dbsp/src/operator/communication/exchange.rs @@ -14,7 +14,7 @@ use crate::{ EXCHANGE_WAIT_TIME_SECONDS, INPUT_BATCHES_STATS, MetaItem, OUTPUT_BATCHES_STATS, OperatorLocation, OperatorMeta, }, - operator_traits::{Operator, SinkOperator, SourceOperator}, + operator_traits::{Operator, OperatorName, SinkOperator, SourceOperator}, runtime::{WorkerLocation, WorkerLocations}, tokio::TOKIO, }, @@ -40,7 +40,7 @@ use std::{ ops::Range, pin::Pin, sync::{ - Arc, Mutex, MutexGuard, OnceLock, RwLock, + Arc, Mutex, MutexGuard, RwLock, atomic::{AtomicIsize, AtomicPtr, AtomicU64, AtomicUsize, Ordering}, }, time::{Duration, Instant, SystemTime}, @@ -331,7 +331,7 @@ impl ExchangeClient { pub type ExchangeId = u32; pub trait ExchangeDelivery { - fn name(&self) -> &str; + fn name(&self) -> Arc; fn received<'a>( &'a self, @@ -618,7 +618,7 @@ pub(crate) struct Exchange { exchange_id: ExchangeId, /// Identifies the `ExchangeReceiver` operator for use in profile data. - receiver_global_node_id: OnceLock>, + name: OperatorName, /// The number of communicating peers. npeers: usize, @@ -756,7 +756,7 @@ where let exchange = Arc::new(Self { exchange_id, - receiver_global_node_id: Default::default(), + name: OperatorName::new("ExchangeReceiver"), npeers, local_workers: layout.local_workers(), clients, @@ -1026,8 +1026,8 @@ impl ExchangeDelivery for Exchange where T: Clone + Debug + Send + 'static, { - fn name(&self) -> &str { - self.receiver_global_node_id.get().unwrap() + fn name(&self) -> Arc { + self.name.get() } fn received<'a>( @@ -1431,10 +1431,7 @@ where } fn init(&mut self, global_id: &GlobalNodeId) { - let _ = self.exchange.receiver_global_node_id.set(Arc::new(format!( - "ExchangeReceiver {}", - global_id.node_identifier() - ))); + self.exchange.name.init(global_id); } fn location(&self) -> OperatorLocation { diff --git a/crates/dbsp/src/operator/dynamic/accumulate_trace.rs b/crates/dbsp/src/operator/dynamic/accumulate_trace.rs index 0fef3ebbf65..f544dda92ff 100644 --- a/crates/dbsp/src/operator/dynamic/accumulate_trace.rs +++ b/crates/dbsp/src/operator/dynamic/accumulate_trace.rs @@ -1,6 +1,6 @@ use crate::circuit::circuit_builder::{StreamId, register_replay_stream}; use crate::circuit::metadata::{INPUT_RECORDS_COUNT, MEMORY_ALLOCATIONS_COUNT, RETAINMENT_BOUNDS}; -use crate::circuit::splitter_output_chunk_size; +use crate::circuit::{NodeId, splitter_output_chunk_size}; use crate::dynamic::{Factory, Weight, WeightTrait}; use crate::operator::dynamic::trace::{DelayedTraceId, TraceBounds}; use crate::operator::{TraceBound, require_persistent_id}; @@ -876,7 +876,8 @@ impl ReplayState { pub struct AccumulateZ1Trace { // For error reporting. - global_id: GlobalNodeId, + local_id: NodeId, + name: OperatorName, time: T::Time, trace: Option, replay_state: Option>, @@ -909,7 +910,8 @@ where bounds: TraceBounds, ) -> Self { Self { - global_id: GlobalNodeId::root(), + local_id: NodeId::root(), + name: OperatorName::new("AccumulateZ1Trace"), time: ::clock_start(), trace: None, replay_state: None, @@ -952,11 +954,8 @@ where /// replay, `replay_stream` is active while the operator that normally write to /// stream is disabled. pub fn prepare_replay_stream(&mut self, stream: &Stream) -> Stream { - let replay_stream = Stream::with_value( - stream.circuit().clone(), - self.global_id.local_node_id().unwrap(), - stream.value(), - ); + let replay_stream = + Stream::with_value(stream.circuit().clone(), self.local_id, stream.value()); self.delta_stream = Some(replay_stream.clone()); replay_stream @@ -977,7 +976,7 @@ where self.dirty[scope as usize] = false; if scope == 0 && self.trace.is_none() { - self.trace = Some(T::new(&self.trace_factories)); + self.trace = Some(T::new(&self.trace_factories, self.name.get())); } } @@ -992,7 +991,8 @@ where } fn init(&mut self, global_id: &GlobalNodeId) { - self.global_id = global_id.clone(); + self.name.init(global_id); + self.local_id = global_id.local_node_id().unwrap(); } fn metadata(&self, meta: &mut OperatorMeta) { @@ -1032,7 +1032,7 @@ where pid: Option<&str>, files: &mut Vec>, ) -> Result<(), Error> { - let pid = require_persistent_id(pid, &self.global_id)?; + let pid = require_persistent_id(pid, &self.name)?; self.trace .as_mut() .map(|trace| trace.save(base, pid, files)) @@ -1040,7 +1040,7 @@ where } fn restore(&mut self, base: &StoragePath, pid: Option<&str>) -> Result<(), Error> { - let pid = require_persistent_id(pid, &self.global_id)?; + let pid = require_persistent_id(pid, &self.name)?; self.trace .as_mut() @@ -1050,7 +1050,7 @@ where fn clear_state(&mut self) -> Result<(), Error> { // println!("AccumulateZ1Trace-{}::clear_state", &self.global_id); - self.trace = Some(T::new(&self.trace_factories)); + self.trace = Some(T::new(&self.trace_factories, self.name.get())); self.replay_state = None; self.dirty = vec![false; self.root_scope as usize + 1]; @@ -1071,7 +1071,7 @@ where .trace .take() .expect("AccumulateZ1Trace::start_replay: no trace"); - self.trace = Some(T::new(&self.trace_factories)); + self.trace = Some(T::new(&self.trace_factories, self.name.get())); //println!("AccumulateZ1Trace-{}::initializing replay_state", &self.global_id); @@ -1239,7 +1239,7 @@ where } } -use crate::circuit::operator_traits::UnaryOperator; +use crate::circuit::operator_traits::{OperatorName, UnaryOperator}; pub struct AccumulateDelayTrace where diff --git a/crates/dbsp/src/operator/dynamic/accumulator.rs b/crates/dbsp/src/operator/dynamic/accumulator.rs index 90fa0540c8b..1e387460726 100644 --- a/crates/dbsp/src/operator/dynamic/accumulator.rs +++ b/crates/dbsp/src/operator/dynamic/accumulator.rs @@ -13,14 +13,14 @@ use typedmap::TypedMapKey; use crate::{ Circuit, Error, NumEntries, Runtime, Scope, Stream, circuit::{ - LocalStoreMarker, + GlobalNodeId, LocalStoreMarker, circuit_builder::StreamId, metadata::{ ALLOCATED_MEMORY_BYTES, BatchSizeStats, INPUT_BATCHES_STATS, MEMORY_ALLOCATIONS_COUNT, MetaItem, OUTPUT_BATCHES_STATS, OperatorLocation, OperatorMeta, SHARED_MEMORY_BYTES, STATE_RECORDS_COUNT, USED_MEMORY_BYTES, }, - operator_traits::{Operator, UnaryOperator}, + operator_traits::{Operator, OperatorName, UnaryOperator}, }, circuit_cache_key, trace::{Batch, BatchReader, Spine, Trace}, @@ -83,6 +83,7 @@ where B: Batch, { factories: B::Factories, + name: OperatorName, state: Spine, flush: bool, location: &'static Location<'static>, @@ -135,9 +136,11 @@ where } }; + let name = OperatorName::new("Accumulator"); Self { factories: factories.clone(), - state: Spine::new(factories), + state: Spine::new(factories, name.get()), + name, flush: false, location, input_batch_stats: BatchSizeStats::new(), @@ -156,6 +159,11 @@ where Cow::Borrowed("Accumulator") } + fn init(&mut self, global_id: &GlobalNodeId) { + self.name.init(global_id); + self.state.set_name(self.name.get()); + } + fn location(&self) -> OperatorLocation { Some(self.location) } @@ -193,7 +201,7 @@ where /// Clear the operator's state. fn clear_state(&mut self) -> Result<(), Error> { - self.state = Spine::new(&self.factories); + self.state = Spine::new(&self.factories, self.name.get()); self.flush = false; Ok(()) } @@ -239,7 +247,7 @@ where self.flush = false; self.enabled_during_current_transaction = None; - let mut spine = Spine::::new(&self.factories); + let mut spine = Spine::::new(&self.factories, self.name.get()); std::mem::swap(&mut self.state, &mut spine); self.output_batch_stats.add_batch(spine.len()); diff --git a/crates/dbsp/src/operator/dynamic/balance/rebalancing_accumulator.rs b/crates/dbsp/src/operator/dynamic/balance/rebalancing_accumulator.rs index 1c1ae1aa87b..1e8ef0f8a32 100644 --- a/crates/dbsp/src/operator/dynamic/balance/rebalancing_accumulator.rs +++ b/crates/dbsp/src/operator/dynamic/balance/rebalancing_accumulator.rs @@ -5,6 +5,7 @@ use size_of::SizeOf; use crate::{ Circuit, Error, NumEntries, Scope, Stream, circuit::{ + GlobalNodeId, checkpointer::EmptyCheckpoint, circuit_builder::RefStreamValue, metadata::{ @@ -12,7 +13,7 @@ use crate::{ MetaItem, OUTPUT_BATCHES_STATS, OperatorLocation, OperatorMeta, SHARED_MEMORY_BYTES, STATE_RECORDS_COUNT, USED_MEMORY_BYTES, }, - operator_traits::{Operator, UnaryOperator}, + operator_traits::{Operator, OperatorName, UnaryOperator}, }, trace::{Batch, BatchReader, Spine, Trace}, }; @@ -51,6 +52,7 @@ where B: Batch, { factories: B::Factories, + name: OperatorName, state: Spine, flush: bool, location: &'static Location<'static>, @@ -69,9 +71,11 @@ where B: Batch, { pub fn new(factories: &B::Factories, location: &'static Location<'static>) -> Self { + let name = OperatorName::new("RebalancingAccumulator"); Self { factories: factories.clone(), - state: Spine::new(factories), + state: Spine::new(factories, name.get()), + name, flush: false, location, input_batch_stats: BatchSizeStats::new(), @@ -81,7 +85,7 @@ where } pub fn clear_state(&mut self) { - self.state = Spine::new(&self.factories); + self.state = Spine::new(&self.factories, self.name.get()); self.flush = false; } @@ -111,7 +115,14 @@ where B: Batch, { fn name(&self) -> std::borrow::Cow<'static, str> { - Cow::Borrowed("BalancingAccumulator") + Cow::Borrowed("Balancing Accumulator") + } + + fn init(&mut self, global_id: &GlobalNodeId) { + let mut inner = self.0.borrow_mut(); + inner.name.init(global_id); + let name = inner.name.get(); + inner.state.set_name(name); } fn location(&self) -> OperatorLocation { @@ -151,9 +162,10 @@ where /// Clear the operator's state. fn clear_state(&mut self) -> Result<(), Error> { - let state = Spine::new(&self.0.borrow().factories); - self.0.borrow_mut().state = state; - self.0.borrow_mut().flush = false; + let mut inner = self.0.borrow_mut(); + let state = Spine::new(&inner.factories, inner.name.get()); + inner.state = state; + inner.flush = false; Ok(()) } @@ -186,7 +198,7 @@ where let result = if inner.flush { inner.flush = false; - let mut spine = Spine::::new(&inner.factories); + let mut spine = Spine::::new(&inner.factories, inner.name.get()); std::mem::swap(&mut inner.state, &mut spine); inner.output_batch_stats.add_batch(spine.len()); @@ -220,7 +232,7 @@ where let result = if inner.flush { inner.flush = false; - let mut spine = Spine::::new(&inner.factories); + let mut spine = Spine::::new(&inner.factories, inner.name.get()); std::mem::swap(&mut inner.state, &mut spine); inner.output_batch_stats.add_batch(spine.len()); diff --git a/crates/dbsp/src/operator/dynamic/group/test.rs b/crates/dbsp/src/operator/dynamic/group/test.rs index d64d7fabd63..0c4f9326c45 100644 --- a/crates/dbsp/src/operator/dynamic/group/test.rs +++ b/crates/dbsp/src/operator/dynamic/group/test.rs @@ -1,6 +1,6 @@ #![allow(clippy::type_complexity)] -use std::cmp::Ordering; +use std::{cmp::Ordering, sync::Arc}; use crate::{ DBData, DynZWeight, RootCircuit, Runtime, ZWeight, @@ -380,7 +380,7 @@ fn lead_test(trace: Vec>, transaction: bool) { ) .unwrap(); - let mut ref_trace = TestBatch::new(&TestBatchFactories::new()); + let mut ref_trace = TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))); if transaction { dbsp.start_transaction().unwrap(); @@ -435,7 +435,7 @@ fn lag_test(trace: Vec>, transaction: bool) { ) .unwrap(); - let mut ref_trace = TestBatch::new(&TestBatchFactories::new()); + let mut ref_trace = TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))); if transaction { dbsp.start_transaction().unwrap(); @@ -821,7 +821,7 @@ fn test_topk(trace: Vec>, transaction: bool) { ) .unwrap(); - let mut ref_trace = TestBatch::new(&TestBatchFactories::new()); + let mut ref_trace = TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))); if transaction { dbsp.start_transaction().unwrap(); diff --git a/crates/dbsp/src/operator/dynamic/join.rs b/crates/dbsp/src/operator/dynamic/join.rs index 25dec2cf238..017fd01530e 100644 --- a/crates/dbsp/src/operator/dynamic/join.rs +++ b/crates/dbsp/src/operator/dynamic/join.rs @@ -4,6 +4,7 @@ use crate::circuit::metadata::{ BatchSizeStats, COMPUTED_OUTPUT_RECORDS_COUNT, LEFT_INPUT_RECORDS_COUNT, OUTPUT_BATCHES_STATS, OUTPUT_REDUNDANCY_PERCENT, RIGHT_INPUT_INTEGRAL_RECORDS_COUNT, }; +use crate::circuit::operator_traits::OperatorName; use crate::circuit::splitter_output_chunk_size; use crate::dynamic::DynData; use crate::operator::async_stream_operators::{StreamingBinaryOperator, StreamingBinaryWrapper}; @@ -1262,6 +1263,7 @@ where T: ZBatchReader, Z: IndexedZSet, { + name: OperatorName, right_factories: T::Factories, output_factories: Z::Factories, timed_item_factory: @@ -1322,6 +1324,7 @@ where clock: Clk, ) -> Self { Self { + name: OperatorName::new("JoinTrace"), right_factories: right_factories.clone(), output_factories: output_factories.clone(), timed_item_factory, @@ -1353,6 +1356,10 @@ where Cow::Borrowed("JoinTrace") } + fn init(&mut self, global_id: &crate::circuit::GlobalNodeId) { + self.name.init(global_id); + } + fn location(&self) -> OperatorLocation { Some(self.location) } @@ -1685,7 +1692,7 @@ where start += run_length; if let Entry::Vacant(vacant) = self.future_outputs.borrow_mut().entry(batch_time) { - let mut spine = as Trace>::new(&self.output_factories); + let mut spine = as Trace>::new(&self.output_factories, self.name.get()); spine.insert(Z::dyn_from_tuples(&self.output_factories, (), &mut batch)).await; vacant.insert(spine); }; diff --git a/crates/dbsp/src/operator/dynamic/multijoin/match_keys.rs b/crates/dbsp/src/operator/dynamic/multijoin/match_keys.rs index ce2414876de..3ad79e60d59 100644 --- a/crates/dbsp/src/operator/dynamic/multijoin/match_keys.rs +++ b/crates/dbsp/src/operator/dynamic/multijoin/match_keys.rs @@ -240,9 +240,7 @@ where } pub fn build(self) -> Match { - let id = self.global_id.local_node_id().unwrap(); Match { - global_id: self.global_id, labels: BTreeMap::new(), prefix: None, prefix_stream: self.prefix_stream, @@ -252,11 +250,12 @@ where flush: Cell::new(false), async_stream: None, inner: Rc::new(MatchInternal::new( - id, + &self.global_id, self.factories, self.join_func, self.circuit, )), + global_id: self.global_id, } } } @@ -424,9 +423,8 @@ where O: IndexedZSet, F: MatchFunc, { - // Used in debug prints. - #[allow(dead_code)] - id: NodeId, + /// For profiling and debug prints. + name: Arc, key_factory: &'static dyn Factory, output_factories: O::Factories, timed_item_factory: @@ -455,9 +453,14 @@ where O: IndexedZSet, F: MatchFunc, { - fn new(id: NodeId, factories: MatchFactories, join_func: F, circuit: C) -> Self { + fn new( + global_id: &GlobalNodeId, + factories: MatchFactories, + join_func: F, + circuit: C, + ) -> Self { Self { - id, + name: Arc::new(format!("Match {}", global_id.node_identifier())), key_factory: factories.prefix_factories.key_factory(), output_factories: factories.output_factories, timed_item_factory: factories.timed_item_factory, @@ -468,7 +471,7 @@ where future_outputs: RefCell::new(HashMap::new()), stats: RefCell::new(MatchStats::new()), circuit: circuit.clone(), - output_stream: Stream::new(circuit, id), + output_stream: Stream::new(circuit, global_id.local_node_id().unwrap()), phantom: PhantomData, } } @@ -556,7 +559,7 @@ where start += run_length; if let Entry::Vacant(vacant) = self.future_outputs.borrow_mut().entry(batch_time) { - let mut spine = as Trace>::new(&self.output_factories); + let mut spine = as Trace>::new(&self.output_factories, self.name.clone()); spine.insert(O::dyn_from_tuples(&self.output_factories, (), &mut batch)).await; vacant.insert(spine); } diff --git a/crates/dbsp/src/operator/dynamic/neighborhood.rs b/crates/dbsp/src/operator/dynamic/neighborhood.rs index ebbcb9e28c3..94be16d53d2 100644 --- a/crates/dbsp/src/operator/dynamic/neighborhood.rs +++ b/crates/dbsp/src/operator/dynamic/neighborhood.rs @@ -598,7 +598,10 @@ mod test { use anyhow::Result as AnyResult; use feldera_storage::tokio::TOKIO; use proptest::{collection::vec, prelude::*}; - use std::cmp::{max, min}; + use std::{ + cmp::{max, min}, + sync::Arc, + }; impl TestBatch { fn neighborhood( @@ -652,7 +655,7 @@ mod test { TestBatch::from_data(output.as_slice()) } else { - TestBatch::new(&TestBatchFactories::new()) + TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))) } } } @@ -914,7 +917,7 @@ mod test { let (mut dbsp, (descr_handle, input_handle, output_handle)) = Runtime::init_circuit(4, test_circuit).unwrap(); - let mut ref_trace = TestBatch::new(&TestBatchFactories::new()); + let mut ref_trace = TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))); for (batch, (start_key, start_val), before, after) in trace.into_iter() { diff --git a/crates/dbsp/src/operator/dynamic/sample.rs b/crates/dbsp/src/operator/dynamic/sample.rs index c5cdbe81f4b..ed5e94726fb 100644 --- a/crates/dbsp/src/operator/dynamic/sample.rs +++ b/crates/dbsp/src/operator/dynamic/sample.rs @@ -349,7 +349,7 @@ mod test { use anyhow::Result as AnyResult; use feldera_storage::tokio::TOKIO; use proptest::{collection::vec, prelude::*}; - use std::{cmp::Ordering, collections::BTreeSet}; + use std::{cmp::Ordering, collections::BTreeSet, sync::Arc}; fn test_circuit( circuit: &mut RootCircuit, @@ -454,7 +454,7 @@ mod test { let (mut dbsp, (sample_size_handle, input_handle, output_sample_handle, output_quantile_handle)) = Runtime::init_circuit(4, test_circuit).unwrap(); - let mut ref_trace: TestBatch*/, DynData/**/, (), DynZWeight> = TestBatch::new(&TestBatchFactories::new()); + let mut ref_trace: TestBatch*/, DynData/**/, (), DynZWeight> = TestBatch::new(&TestBatchFactories::new(), Arc::new(String::from("Test"))); for (batch, sample_size) in trace.into_iter() { diff --git a/crates/dbsp/src/operator/dynamic/sharded_accumulator.rs b/crates/dbsp/src/operator/dynamic/sharded_accumulator.rs index ec7b5006ed7..e000e8762e5 100644 --- a/crates/dbsp/src/operator/dynamic/sharded_accumulator.rs +++ b/crates/dbsp/src/operator/dynamic/sharded_accumulator.rs @@ -5,7 +5,7 @@ use std::{ ops::Range, panic::Location, pin::Pin, - sync::{Arc, Mutex, MutexGuard, OnceLock}, + sync::{Arc, Mutex, MutexGuard}, }; use feldera_samply::Span; @@ -23,7 +23,7 @@ use crate::{ MetaItem, OUTPUT_BATCHES_STATS, OperatorLocation, OperatorMeta, SHARED_MEMORY_BYTES, SPINE_COUNT, STATE_RECORDS_COUNT, USED_MEMORY_BYTES, }, - operator_traits::{Operator, SinkOperator, SourceOperator}, + operator_traits::{Operator, OperatorName, SinkOperator, SourceOperator}, }, circuit_cache_key, operator::{ @@ -97,7 +97,7 @@ struct ShardedAccumulator where B: Batch, { - receiver_global_node_id: OnceLock>, + name: OperatorName, exchange_id: ExchangeId, /// The number of communicating peers. @@ -159,17 +159,26 @@ where let layout = runtime.layout(); let npeers = layout.n_workers(); + let name = OperatorName::new("ShardedAccumulatorReceiver"); let exchange = Arc::new(Self { exchange_id, - receiver_global_node_id: Default::default(), npeers, local_workers: layout.local_workers(), factories: factories.clone(), clients, rxq: layout .local_workers() - .map(|_| Mutex::new(Rxq::new(runtime, worker_index, factories, npeers))) + .map(|_| { + Mutex::new(Rxq::new( + runtime, + worker_index, + factories, + npeers, + name.get(), + )) + }) .collect(), + name, }); directory.insert(exchange_id, exchange.clone()); @@ -204,7 +213,7 @@ where } } - async fn send(self: &Arc, global_node_id: &Arc, batch: B, flush: bool) { + async fn send(self: &Arc, name: Arc, batch: B, flush: bool) { let sender = Runtime::worker_index(); let runtime = Runtime::runtime().unwrap(); @@ -264,7 +273,7 @@ where .collect_vec(); let this = self.clone(); if let Some(waiter) = this.clients.connect(receivers.start).await.send( - global_node_id.clone(), + name.clone(), this.exchange_id, sender, items, @@ -280,7 +289,7 @@ where .with_category("Exchange") .with_tooltip(|| { format!( - "{global_node_id} wait for batches to merge in {} receive queues (for workers {})", + "{name} wait for batches to merge in {} receive queues (for workers {})", local_waiters.len(), local_waiters .iter() @@ -297,7 +306,7 @@ where .with_category("Exchange") .with_tooltip(|| { format!( - "{global_node_id} wait for {} to drain from {} tx buffers", + "{name} wait for {} to drain from {} tx buffers", HumanBytes::from(serialized_bytes), remote_waiters.len() ) @@ -312,14 +321,21 @@ where let receiver = Runtime::worker_index(); self.rxq(receiver).receive() } + + fn set_name(&self, global_id: &GlobalNodeId) { + self.name.init(global_id); + for rxq in &self.rxq { + rxq.lock().unwrap().set_name(self.name.get()); + } + } } impl ExchangeDelivery for ShardedAccumulator where B: Batch