Skip to content

Commit 5f95eb7

Browse files
committed
refactor(engine): replace long argument lists with params structs
Convert cluster (Calvin scheduler driver, raft startup), security scope grant/expiry, DDL scope-grant, trace export, gateway dispatch, and Data-Plane core spawn to take dedicated params structs instead of long positional argument lists, updating all call sites accordingly. Also split the core health watchdog out of data/runtime.rs into its own data/core_health.rs module.
1 parent d803f65 commit 5f95eb7

17 files changed

Lines changed: 454 additions & 277 deletions

File tree

nodedb/src/bootstrap/data_plane.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::bridge::quiesce::CollectionQuiesce;
1212
use crate::control::array_catalog::ArrayCatalog;
1313
use crate::control::metrics::SystemMetrics;
1414
use crate::data::eventfd::EventFdNotifier;
15-
use crate::data::runtime::{CoreCompactionConfig, spawn_core};
15+
use crate::data::runtime::{CoreCompactionConfig, SpawnCoreParams, spawn_core};
1616
use crate::event::EventProducer;
1717
use crate::storage::quarantine::QuarantineRegistry;
1818

@@ -91,24 +91,24 @@ pub fn spawn_data_plane_cores(
9191
for (core_id, (data_side, event_producer)) in
9292
data_sides.into_iter().zip(event_producers).enumerate()
9393
{
94-
let (handle, notifier) = spawn_core(
94+
let (handle, notifier) = spawn_core(SpawnCoreParams {
9595
core_id,
96-
data_side.request_rx,
97-
data_side.response_tx,
98-
&config.server.data_dir,
99-
Arc::clone(&wal_records),
100-
replay_tombstones.clone(),
96+
request_rx: data_side.request_rx,
97+
response_tx: data_side.response_tx,
98+
data_dir: &config.server.data_dir,
99+
wal_records: Arc::clone(&wal_records),
100+
tombstones: replay_tombstones.clone(),
101101
num_cores,
102-
compaction_cfg.clone(),
103-
Some(Arc::clone(&system_metrics)),
104-
Some(event_producer),
105-
Arc::clone(&governor),
106-
Some(Arc::clone(&quiesce)),
107-
Arc::clone(&hlc),
108-
Arc::clone(&array_catalog),
109-
Arc::clone(&quarantine_registry),
110-
Arc::clone(&maintenance_budget),
111-
)?;
102+
compaction_config: compaction_cfg.clone(),
103+
system_metrics: Some(Arc::clone(&system_metrics)),
104+
event_producer: Some(event_producer),
105+
governor: Arc::clone(&governor),
106+
quiesce: Some(Arc::clone(&quiesce)),
107+
hlc: Arc::clone(&hlc),
108+
array_catalog: Arc::clone(&array_catalog),
109+
quarantine_registry: Arc::clone(&quarantine_registry),
110+
maintenance_budget: Arc::clone(&maintenance_budget),
111+
})?;
112112
core_handles.push(handle);
113113
notifiers.push((core_id, notifier));
114114
}

nodedb/src/control/cluster/calvin/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ pub mod scheduler;
55

66
pub use executor::{OllpConfig, OllpError, OllpOrchestrator};
77
pub use scheduler::{
8-
CalvinReadResultProposal, ReadResultEvent, Scheduler, SchedulerConfig,
8+
CalvinReadResultProposal, ReadResultEvent, Scheduler, SchedulerConfig, SchedulerParams,
99
propose_calvin_read_result,
1010
};

nodedb/src/control/cluster/calvin/scheduler/driver/core/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,4 @@ pub mod scheduler;
3939
mod tests;
4040

4141
pub use propose::{CalvinReadResultProposal, propose_calvin_read_result};
42-
pub use scheduler::Scheduler;
42+
pub use scheduler::{Scheduler, SchedulerParams};

nodedb/src/control/cluster/calvin/scheduler/driver/core/scheduler.rs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,34 @@ pub struct Scheduler {
8989
mpsc::Sender<CompletionItem>,
9090
}
9191

92+
/// Parameters for [`Scheduler::new`].
93+
pub struct SchedulerParams {
94+
pub vshard_id: u32,
95+
pub receiver: mpsc::Receiver<SequencedTxn>,
96+
pub shared: Arc<SharedState>,
97+
pub multi_raft: Arc<Mutex<MultiRaft>>,
98+
pub last_applied_epoch: u64,
99+
pub rebuild_target_epoch: u64,
100+
pub config: SchedulerConfig,
101+
pub metrics: Arc<SchedulerMetrics>,
102+
pub read_result_rx: mpsc::Receiver<ReadResultEvent>,
103+
}
104+
92105
impl Scheduler {
93106
/// Construct a scheduler.
94-
#[allow(clippy::too_many_arguments)]
95-
pub fn new(
96-
vshard_id: u32,
97-
receiver: mpsc::Receiver<SequencedTxn>,
98-
shared: Arc<SharedState>,
99-
multi_raft: Arc<Mutex<MultiRaft>>,
100-
last_applied_epoch: u64,
101-
rebuild_target_epoch: u64,
102-
config: SchedulerConfig,
103-
metrics: Arc<SchedulerMetrics>,
104-
read_result_rx: mpsc::Receiver<ReadResultEvent>,
105-
) -> Self {
107+
pub fn new(params: SchedulerParams) -> Self {
108+
let SchedulerParams {
109+
vshard_id,
110+
receiver,
111+
shared,
112+
multi_raft,
113+
last_applied_epoch,
114+
rebuild_target_epoch,
115+
config,
116+
metrics,
117+
read_result_rx,
118+
} = params;
119+
106120
// Capacity: at most one completion per inflight txn. Use the incoming
107121
// channel capacity as a proxy for the max concurrent pending count.
108122
let completion_cap = config.channel_capacity;

nodedb/src/control/cluster/calvin/scheduler/driver/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ pub mod types;
88

99
pub use barrier::ReadResultEvent;
1010
pub use config::SchedulerConfig;
11-
pub use core::{CalvinReadResultProposal, Scheduler, propose_calvin_read_result};
11+
pub use core::{CalvinReadResultProposal, Scheduler, SchedulerParams, propose_calvin_read_result};

nodedb/src/control/cluster/calvin/scheduler/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub mod metrics;
66
pub mod recovery;
77

88
pub use driver::{
9-
CalvinReadResultProposal, ReadResultEvent, Scheduler, SchedulerConfig,
9+
CalvinReadResultProposal, ReadResultEvent, Scheduler, SchedulerConfig, SchedulerParams,
1010
propose_calvin_read_result,
1111
};
1212
pub use lock_manager::{AcquireOutcome, LockKey, LockManager, TxnId};

nodedb/src/control/cluster/start_raft.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ use crate::control::cluster::handle::ClusterHandle;
2222
use crate::control::cluster::metadata_applier::MetadataCommitApplier;
2323
use crate::control::cluster::snapshot_hook::RaftSnapshotQuarantineHook;
2424
use crate::control::cluster::spsc_applier::SpscCommitApplier;
25-
use crate::control::cluster::start_raft_helpers::{build_vshard_handler, spawn_vshard_schedulers};
25+
use crate::control::cluster::start_raft_helpers::{
26+
SpawnVshardSchedulersParams, build_vshard_handler, spawn_vshard_schedulers,
27+
};
2628
use crate::control::distributed_applier::{
2729
ProposeTracker, create_distributed_applier, run_apply_loop,
2830
};
@@ -325,14 +327,14 @@ pub fn start_raft(
325327
let sequencer_metrics = Arc::clone(&sequencer_service.metrics);
326328

327329
let scheduler_config = SchedulerConfig::default();
328-
spawn_vshard_schedulers(
330+
spawn_vshard_schedulers(SpawnVshardSchedulersParams {
329331
handle,
330-
&shared,
331-
raft_loop_handle.clone(),
332-
&sequencer_state_machine,
333-
&calvin_read_result_senders,
334-
&scheduler_config,
335-
)?;
332+
shared: &shared,
333+
raft_loop_handle: raft_loop_handle.clone(),
334+
sequencer_state_machine: &sequencer_state_machine,
335+
calvin_read_result_senders: &calvin_read_result_senders,
336+
scheduler_config: &scheduler_config,
337+
})?;
336338

337339
let running = tokio::task::block_in_place(|| {
338340
tokio::runtime::Handle::current().block_on(nodedb_cluster::start_cluster_subsystems(

nodedb/src/control/cluster/start_raft_helpers.rs

Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ use nodedb_cluster::wire::VShardEnvelope;
1010

1111
use crate::control::cluster::calvin::scheduler::metrics::SchedulerMetrics;
1212
use crate::control::cluster::calvin::scheduler::read_last_applied_epoch;
13-
use crate::control::cluster::calvin::{ReadResultEvent, Scheduler, SchedulerConfig};
13+
use crate::control::cluster::calvin::{
14+
ReadResultEvent, Scheduler, SchedulerConfig, SchedulerParams,
15+
};
1416
use crate::control::cluster::handle::ClusterHandle;
1517
use crate::control::state::SharedState;
1618

@@ -91,6 +93,17 @@ fn hosted_vshards(routing: &RwLock<nodedb_cluster::RoutingTable>, node_id: u64)
9193
vshards
9294
}
9395

96+
/// Parameters for [`reconcile_vshard_schedulers`].
97+
struct ReconcileSchedulersParams<'a> {
98+
node_id: u64,
99+
routing: &'a Arc<RwLock<nodedb_cluster::RoutingTable>>,
100+
shared: &'a Arc<SharedState>,
101+
raft_loop_handle: &'a Arc<Mutex<nodedb_cluster::multi_raft::MultiRaft>>,
102+
sequencer_state_machine: &'a Arc<Mutex<SequencerStateMachine>>,
103+
calvin_read_result_senders: &'a ReadResultSenders,
104+
scheduler_config: &'a SchedulerConfig,
105+
}
106+
94107
/// Idempotently ensure a Calvin `Scheduler` is running for every vShard this
95108
/// node currently hosts.
96109
///
@@ -103,16 +116,17 @@ fn hosted_vshards(routing: &RwLock<nodedb_cluster::RoutingTable>, node_id: u64)
103116
/// left this node. vShard removal happens via migration / decommission, which
104117
/// own their own teardown path; wiring scheduler removal into that lifecycle is
105118
/// tracked as a separate follow-up.
106-
#[allow(clippy::too_many_arguments)]
107-
fn reconcile_vshard_schedulers(
108-
node_id: u64,
109-
routing: &Arc<RwLock<nodedb_cluster::RoutingTable>>,
110-
shared: &Arc<SharedState>,
111-
raft_loop_handle: &Arc<Mutex<nodedb_cluster::multi_raft::MultiRaft>>,
112-
sequencer_state_machine: &Arc<Mutex<SequencerStateMachine>>,
113-
calvin_read_result_senders: &ReadResultSenders,
114-
scheduler_config: &SchedulerConfig,
115-
) -> crate::Result<usize> {
119+
fn reconcile_vshard_schedulers(params: ReconcileSchedulersParams<'_>) -> crate::Result<usize> {
120+
let ReconcileSchedulersParams {
121+
node_id,
122+
routing,
123+
shared,
124+
raft_loop_handle,
125+
sequencer_state_machine,
126+
calvin_read_result_senders,
127+
scheduler_config,
128+
} = params;
129+
116130
let mut spawned = 0usize;
117131
for vshard_id in hosted_vshards(routing, node_id) {
118132
// Already-served vShards keep their running scheduler untouched.
@@ -139,17 +153,17 @@ fn reconcile_vshard_schedulers(
139153
.unwrap_or_else(|p| p.into_inner())
140154
.insert(vshard_id, read_result_tx);
141155

142-
let scheduler = Scheduler::new(
156+
let scheduler = Scheduler::new(SchedulerParams {
143157
vshard_id,
144-
sequenced_rx,
145-
Arc::clone(shared),
146-
raft_loop_handle.clone(),
158+
receiver: sequenced_rx,
159+
shared: Arc::clone(shared),
160+
multi_raft: raft_loop_handle.clone(),
147161
last_applied_epoch,
148-
last_applied_epoch,
149-
scheduler_config.clone(),
150-
SchedulerMetrics::new(),
162+
rebuild_target_epoch: last_applied_epoch,
163+
config: scheduler_config.clone(),
164+
metrics: SchedulerMetrics::new(),
151165
read_result_rx,
152-
);
166+
});
153167
let shutdown = shared.shutdown.subscribe();
154168
tokio::spawn(async move {
155169
scheduler.run(shutdown).await;
@@ -159,6 +173,16 @@ fn reconcile_vshard_schedulers(
159173
Ok(spawned)
160174
}
161175

176+
/// Parameters for [`spawn_vshard_schedulers`].
177+
pub(super) struct SpawnVshardSchedulersParams<'a> {
178+
pub(super) handle: &'a ClusterHandle,
179+
pub(super) shared: &'a Arc<SharedState>,
180+
pub(super) raft_loop_handle: Arc<Mutex<nodedb_cluster::multi_raft::MultiRaft>>,
181+
pub(super) sequencer_state_machine: &'a Arc<Mutex<SequencerStateMachine>>,
182+
pub(super) calvin_read_result_senders: &'a ReadResultSenders,
183+
pub(super) scheduler_config: &'a SchedulerConfig,
184+
}
185+
162186
/// Spawn Calvin `Scheduler` tasks for this node's vShards and keep the set in
163187
/// sync with cluster membership.
164188
///
@@ -174,28 +198,31 @@ fn reconcile_vshard_schedulers(
174198
/// (covers the bootstrap node, which already sees its membership) and then
175199
/// spawns a background task that re-reconciles on a short interval until
176200
/// shutdown. Reconcile is idempotent and add-only.
177-
#[allow(clippy::too_many_arguments)]
178201
pub(super) fn spawn_vshard_schedulers(
179-
handle: &ClusterHandle,
180-
shared: &Arc<SharedState>,
181-
raft_loop_handle: Arc<Mutex<nodedb_cluster::multi_raft::MultiRaft>>,
182-
sequencer_state_machine: &Arc<Mutex<SequencerStateMachine>>,
183-
calvin_read_result_senders: &ReadResultSenders,
184-
scheduler_config: &SchedulerConfig,
202+
params: SpawnVshardSchedulersParams<'_>,
185203
) -> crate::Result<()> {
204+
let SpawnVshardSchedulersParams {
205+
handle,
206+
shared,
207+
raft_loop_handle,
208+
sequencer_state_machine,
209+
calvin_read_result_senders,
210+
scheduler_config,
211+
} = params;
212+
186213
let node_id = handle.node_id;
187214
let routing = Arc::clone(&handle.routing);
188215

189216
// Initial reconcile: schedulers for vShards this node already knows it hosts.
190-
reconcile_vshard_schedulers(
217+
reconcile_vshard_schedulers(ReconcileSchedulersParams {
191218
node_id,
192-
&routing,
219+
routing: &routing,
193220
shared,
194-
&raft_loop_handle,
221+
raft_loop_handle: &raft_loop_handle,
195222
sequencer_state_machine,
196223
calvin_read_result_senders,
197224
scheduler_config,
198-
)?;
225+
})?;
199226

200227
// Background reconcile: pick up vShards whose membership lands after startup
201228
// (joiner admission) or shifts later (rebalancing). The routing table has no
@@ -215,15 +242,15 @@ pub(super) fn spawn_vshard_schedulers(
215242
biased;
216243
_ = shutdown.wait_cancelled() => break,
217244
_ = tick.tick() => {
218-
if let Err(e) = reconcile_vshard_schedulers(
245+
if let Err(e) = reconcile_vshard_schedulers(ReconcileSchedulersParams {
219246
node_id,
220-
&routing,
221-
&shared_task,
222-
&raft_loop_handle,
223-
&sm_task,
224-
&rr_task,
225-
&cfg_task,
226-
) {
247+
routing: &routing,
248+
shared: &shared_task,
249+
raft_loop_handle: &raft_loop_handle,
250+
sequencer_state_machine: &sm_task,
251+
calvin_read_result_senders: &rr_task,
252+
scheduler_config: &cfg_task,
253+
}) {
227254
tracing::warn!(node_id, error = %e, "calvin scheduler reconcile pass failed");
228255
}
229256
}

nodedb/src/control/exec_receiver/executor.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use nodedb_cluster::rpc_codec::{ExecuteRequest, ExecuteResponse, TypedClusterErr
2323

2424
use crate::control::server::exchange::execute_plan_all_local_cores;
2525
use crate::control::state::SharedState;
26+
use crate::control::trace_export::EmitSpanParams;
2627
use crate::types::DatabaseId;
2728
use nodedb_physical::physical_plan::wire as plan_wire;
2829

@@ -52,15 +53,15 @@ impl PlanExecutor for LocalPlanExecutor {
5253
// Emit one OTLP executor span per leaseholder so the gateway's
5354
// upstream span joins the N leaseholder spans into a single
5455
// distributed trace via the shared `trace_id`.
55-
exporter.emit(
56-
"executor.execute_plan",
56+
exporter.emit(EmitSpanParams {
57+
span_name: "executor.execute_plan",
5758
trace_id,
5859
start,
59-
SystemTime::now(),
60+
end: SystemTime::now(),
6061
tenant_id,
61-
0,
62-
resp.success,
63-
);
62+
vshard_id: 0,
63+
status_ok: resp.success,
64+
});
6465
resp
6566
}
6667

@@ -78,15 +79,15 @@ impl PlanExecutor for LocalPlanExecutor {
7879
.execute_plan_streaming_inner(req, sink)
7980
.instrument(span)
8081
.await;
81-
exporter.emit(
82-
"executor.execute_plan_streaming",
82+
exporter.emit(EmitSpanParams {
83+
span_name: "executor.execute_plan_streaming",
8384
trace_id,
8485
start,
85-
SystemTime::now(),
86+
end: SystemTime::now(),
8687
tenant_id,
87-
0,
88-
outcome.is_none(),
89-
);
88+
vshard_id: 0,
89+
status_ok: outcome.is_none(),
90+
});
9091
outcome
9192
}
9293
}

0 commit comments

Comments
 (0)