Replace Worker list revisions with ordered subscriptions
This commit is contained in:
@@ -1334,7 +1334,6 @@ async fn runtime_protocol_ws_session(
|
||||
request_id,
|
||||
subscription_id: subscription_id.clone(),
|
||||
selector: selector.clone(),
|
||||
snapshot_revision: subscription.snapshot_revision(),
|
||||
snapshot: subscription.snapshot().clone(),
|
||||
},
|
||||
));
|
||||
@@ -1354,7 +1353,6 @@ async fn runtime_protocol_ws_session(
|
||||
SubscriptionFramePayload::Event(
|
||||
SubscriptionEvent::Event {
|
||||
subscription_id: event_subscription_id.clone(),
|
||||
subject_revision: update.subject_revision,
|
||||
payload: update.payload,
|
||||
},
|
||||
),
|
||||
|
||||
@@ -36,5 +36,8 @@ pub mod workspace_request;
|
||||
#[cfg(feature = "fs-store")]
|
||||
pub use fs_store::{FsRuntimeStore, FsRuntimeStoreOptions};
|
||||
pub use management::RuntimeOptions;
|
||||
pub use runtime::{Runtime, RuntimeWorkspaceScope};
|
||||
pub use runtime::{
|
||||
Runtime, RuntimeEventSelectorSubscription, RuntimeSubscriptionRecvError,
|
||||
RuntimeSubscriptionUpdate, RuntimeWorkspaceScope,
|
||||
};
|
||||
pub use session_store::UploadedFileUploadContext;
|
||||
|
||||
@@ -151,7 +151,6 @@ impl RuntimeWorkerRestoreResult {
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct RuntimeSubscriptionUpdate {
|
||||
pub subject_revision: u64,
|
||||
pub payload: SubscriptionEventPayload,
|
||||
}
|
||||
|
||||
@@ -168,7 +167,6 @@ pub enum RuntimeSubscriptionRecvError {
|
||||
pub struct RuntimeEventSelectorSubscription {
|
||||
subscription_id: u64,
|
||||
selector: EventSubscriptionSelector,
|
||||
snapshot_revision: u64,
|
||||
snapshot: SubscriptionSnapshot,
|
||||
receiver: mpsc::Receiver<RuntimeSubscriptionUpdate>,
|
||||
lagged: Arc<AtomicBool>,
|
||||
@@ -180,10 +178,6 @@ impl RuntimeEventSelectorSubscription {
|
||||
&self.selector
|
||||
}
|
||||
|
||||
pub fn snapshot_revision(&self) -> u64 {
|
||||
self.snapshot_revision
|
||||
}
|
||||
|
||||
pub fn snapshot(&self) -> &SubscriptionSnapshot {
|
||||
&self.snapshot
|
||||
}
|
||||
@@ -1280,7 +1274,6 @@ impl Runtime {
|
||||
state.persist_runtime_snapshot()?;
|
||||
}
|
||||
let snapshot = state.subscription_snapshot(scope, &selector)?;
|
||||
let snapshot_revision = state.subscription_revision;
|
||||
let subscription_id = state.next_event_subscription_id;
|
||||
state.next_event_subscription_id = state.next_event_subscription_id.saturating_add(1);
|
||||
let (sender, receiver) = mpsc::channel(SUBSCRIPTION_QUEUE_CAPACITY);
|
||||
@@ -1298,7 +1291,6 @@ impl Runtime {
|
||||
Ok(RuntimeEventSelectorSubscription {
|
||||
subscription_id,
|
||||
selector,
|
||||
snapshot_revision,
|
||||
snapshot,
|
||||
receiver,
|
||||
lagged,
|
||||
@@ -2217,9 +2209,9 @@ impl Runtime {
|
||||
if let Some(payload) = input_protocol_event(&input) {
|
||||
state.push_worker_observation_event(worker_ref.clone(), payload);
|
||||
}
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
|
||||
Ok(WorkerInteractionAck {
|
||||
worker_ref: worker_ref.clone(),
|
||||
@@ -2500,9 +2492,9 @@ impl Runtime {
|
||||
worker.workdir_attachments = workdir_attachments;
|
||||
worker.detail()
|
||||
};
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
Ok(detail)
|
||||
}
|
||||
|
||||
@@ -2541,8 +2533,8 @@ impl Runtime {
|
||||
state
|
||||
.observation_events
|
||||
.retain(|event| event.worker_ref != *worker_ref);
|
||||
state.publish_worker_removed(worker_ref.worker_id, workspace_id.as_deref())?;
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.publish_worker_removed(worker_ref.worker_id, workspace_id.as_deref())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -2564,9 +2556,9 @@ impl Runtime {
|
||||
let mut state = self.lock()?;
|
||||
let worker = state.worker_mut(worker_ref)?;
|
||||
worker.apply_worker_state(&snapshot);
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2870,8 +2862,8 @@ impl Runtime {
|
||||
state
|
||||
.observation_events
|
||||
.retain(|event| event.worker_ref != *worker_ref);
|
||||
state.publish_worker_removed(worker_ref.worker_id, removed_workspace_id.as_deref())?;
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.publish_worker_removed(worker_ref.worker_id, removed_workspace_id.as_deref())?;
|
||||
Ok(WorkerDeleteResult {
|
||||
worker_id: removed.worker_id,
|
||||
deleted: true,
|
||||
@@ -2959,13 +2951,13 @@ impl Runtime {
|
||||
let worker_state_changed =
|
||||
state.project_protocol_event_to_worker_state(worker_ref, &payload);
|
||||
let activity_changed = state.project_internal_worker_activity(worker_ref, &payload);
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed {
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
let event = state.push_worker_observation_event(worker_ref.clone(), payload);
|
||||
Ok(event)
|
||||
}
|
||||
@@ -3021,13 +3013,13 @@ impl Runtime {
|
||||
state.project_protocol_event_to_worker_state(&worker_ref, &payload);
|
||||
let activity_changed =
|
||||
state.project_internal_worker_activity(&worker_ref, &payload);
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed {
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
Ok(state.push_worker_observation_event(worker_ref, payload))
|
||||
}),
|
||||
)
|
||||
@@ -3054,13 +3046,13 @@ impl Runtime {
|
||||
state.project_protocol_event_to_worker_state(&worker_ref, &payload);
|
||||
let activity_changed =
|
||||
state.project_internal_worker_activity(&worker_ref, &payload);
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed {
|
||||
state.persist_runtime_snapshot()?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
}
|
||||
if worker_state_changed || activity_changed {
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
}
|
||||
Ok(state.push_worker_observation_event(worker_ref, payload))
|
||||
}),
|
||||
)
|
||||
@@ -3454,8 +3446,8 @@ impl Runtime {
|
||||
logical_workdir_attachments_from_statuses(&workdir_attachments, claims);
|
||||
candidate.workdir_attachments = workdir_attachments;
|
||||
state.workers.insert(worker_ref.worker_id, candidate);
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
state.persist_worker(&worker_ref.worker_id)?;
|
||||
state.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -3702,8 +3694,6 @@ struct RuntimeState {
|
||||
workspace_config_latest: BTreeMap<String, ConfigBundleRef>,
|
||||
workspace_config_fetch_gates: BTreeMap<String, Arc<Mutex<()>>>,
|
||||
diagnostics: Vec<RuntimeDiagnostic>,
|
||||
subscription_revision: u64,
|
||||
worker_subject_revisions: BTreeMap<WorkerId, u64>,
|
||||
next_event_subscription_id: u64,
|
||||
subscriptions: BTreeMap<u64, SubscriptionSink>,
|
||||
#[cfg(feature = "ws-server")]
|
||||
@@ -3733,8 +3723,6 @@ impl RuntimeState {
|
||||
workspace_config_latest: BTreeMap::new(),
|
||||
workspace_config_fetch_gates: BTreeMap::new(),
|
||||
diagnostics: Vec::new(),
|
||||
subscription_revision: 0,
|
||||
worker_subject_revisions: BTreeMap::new(),
|
||||
next_event_subscription_id: 1,
|
||||
subscriptions: BTreeMap::new(),
|
||||
#[cfg(feature = "ws-server")]
|
||||
@@ -3765,8 +3753,6 @@ impl RuntimeState {
|
||||
workspace_config_latest: BTreeMap::new(),
|
||||
workspace_config_fetch_gates: BTreeMap::new(),
|
||||
diagnostics: Vec::new(),
|
||||
subscription_revision: 0,
|
||||
worker_subject_revisions: BTreeMap::new(),
|
||||
next_event_subscription_id: 1,
|
||||
subscriptions: BTreeMap::new(),
|
||||
#[cfg(feature = "ws-server")]
|
||||
@@ -3895,8 +3881,6 @@ impl RuntimeState {
|
||||
workspace_config_fetch_gates: BTreeMap::new(),
|
||||
workspace_owners: persisted.workspace_owners,
|
||||
diagnostics,
|
||||
subscription_revision: 0,
|
||||
worker_subject_revisions: BTreeMap::new(),
|
||||
next_event_subscription_id: 1,
|
||||
subscriptions: BTreeMap::new(),
|
||||
#[cfg(feature = "ws-server")]
|
||||
@@ -4215,11 +4199,6 @@ impl RuntimeState {
|
||||
runtime_id: None,
|
||||
resource_key: None,
|
||||
availability: protocol::subscription::SubscriptionWorkerAvailability::Observed,
|
||||
subject_revision: self
|
||||
.worker_subject_revisions
|
||||
.get(&worker.worker_id)
|
||||
.copied()
|
||||
.unwrap_or(0),
|
||||
worker_state: worker.worker_state.clone(),
|
||||
state: subscription_worker_state(worker.status),
|
||||
has_running_internal_workers: worker.internal_workers.has_running_worker(),
|
||||
@@ -4232,25 +4211,17 @@ impl RuntimeState {
|
||||
}
|
||||
|
||||
fn publish_worker_upsert(&mut self, worker_id: WorkerId) -> Result<(), RuntimeError> {
|
||||
self.subscription_revision = self.subscription_revision.saturating_add(1);
|
||||
let subject_revision = {
|
||||
let revision = self.worker_subject_revisions.entry(worker_id).or_insert(0);
|
||||
*revision = revision.saturating_add(1);
|
||||
*revision
|
||||
};
|
||||
let worker = self
|
||||
.workers
|
||||
.get(&worker_id)
|
||||
.ok_or(RuntimeError::WorkerNotFound { worker_id })?;
|
||||
let workspace_id = worker.workspace_id.clone();
|
||||
let mut projected = self.subscription_worker(worker)?;
|
||||
projected.subject_revision = subject_revision;
|
||||
let projected = self.subscription_worker(worker)?;
|
||||
let projected_worker_id = projected.worker_id.clone();
|
||||
self.deliver_worker_subscription_update(
|
||||
&projected_worker_id,
|
||||
workspace_id.as_deref(),
|
||||
RuntimeSubscriptionUpdate {
|
||||
subject_revision,
|
||||
payload: SubscriptionEventPayload::WorkerUpserted { worker: projected },
|
||||
},
|
||||
);
|
||||
@@ -4262,19 +4233,12 @@ impl RuntimeState {
|
||||
worker_id: WorkerId,
|
||||
workspace_id: Option<&str>,
|
||||
) -> Result<(), RuntimeError> {
|
||||
self.subscription_revision = self.subscription_revision.saturating_add(1);
|
||||
let subject_revision = {
|
||||
let revision = self.worker_subject_revisions.entry(worker_id).or_insert(0);
|
||||
*revision = revision.saturating_add(1);
|
||||
*revision
|
||||
};
|
||||
let worker_id = SubscriptionWorkerId::new(worker_id.to_string())
|
||||
.map_err(subscription_validation_error)?;
|
||||
self.deliver_worker_subscription_update(
|
||||
&worker_id,
|
||||
workspace_id,
|
||||
RuntimeSubscriptionUpdate {
|
||||
subject_revision,
|
||||
payload: SubscriptionEventPayload::WorkerRemoved {
|
||||
worker_id: worker_id.clone(),
|
||||
runtime_id: None,
|
||||
@@ -4417,8 +4381,8 @@ impl RuntimeState {
|
||||
worker_ref: Some(worker_ref.clone()),
|
||||
});
|
||||
self.workers.insert(worker_ref.worker_id, candidate);
|
||||
self.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
self.persist_runtime_snapshot()?;
|
||||
self.publish_worker_upsert(worker_ref.worker_id)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -6637,7 +6601,6 @@ mod tests {
|
||||
let mut subscription = runtime
|
||||
.subscribe_event_selector(EventSubscriptionSelector::RuntimeWorkers)
|
||||
.unwrap();
|
||||
assert_eq!(subscription.snapshot_revision(), 0);
|
||||
let SubscriptionSnapshot::Workers { workers } = subscription.snapshot() else {
|
||||
panic!("runtime_workers must return a Worker snapshot");
|
||||
};
|
||||
@@ -6645,11 +6608,9 @@ mod tests {
|
||||
|
||||
let created = runtime.create_worker(task_request("live")).unwrap();
|
||||
let update = receive_subscription_update(&mut subscription).unwrap();
|
||||
assert_eq!(update.subject_revision, 1);
|
||||
match update.payload {
|
||||
SubscriptionEventPayload::WorkerUpserted { worker } => {
|
||||
assert_eq!(worker.worker_id.as_str(), created.worker_id.to_string());
|
||||
assert_eq!(worker.subject_revision, 1);
|
||||
assert_eq!(worker.state, SubscriptionWorkerState::Idle);
|
||||
}
|
||||
payload => panic!("unexpected subscription payload: {payload:?}"),
|
||||
@@ -6666,7 +6627,6 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
let update = receive_subscription_update(&mut subscription).unwrap();
|
||||
assert_eq!(update.subject_revision, 2);
|
||||
match update.payload {
|
||||
SubscriptionEventPayload::WorkerUpserted { worker } => {
|
||||
assert_eq!(worker.state, SubscriptionWorkerState::Idle);
|
||||
@@ -6685,7 +6645,6 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
let update = receive_subscription_update(&mut subscription).unwrap();
|
||||
assert_eq!(update.subject_revision, 3);
|
||||
match update.payload {
|
||||
SubscriptionEventPayload::WorkerUpserted { worker } => {
|
||||
assert_eq!(worker.state, SubscriptionWorkerState::Idle);
|
||||
@@ -6727,7 +6686,6 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
let update = receive_subscription_update(&mut subscription).unwrap();
|
||||
assert_eq!(update.subject_revision, 4);
|
||||
match update.payload {
|
||||
SubscriptionEventPayload::WorkerUpserted { worker } => {
|
||||
assert_eq!(worker.state, SubscriptionWorkerState::Idle);
|
||||
@@ -6738,7 +6696,6 @@ mod tests {
|
||||
|
||||
runtime.stop_worker(&created.worker_ref, None).unwrap();
|
||||
let update = receive_subscription_update(&mut subscription).unwrap();
|
||||
assert_eq!(update.subject_revision, 5);
|
||||
match update.payload {
|
||||
SubscriptionEventPayload::WorkerUpserted { worker } => {
|
||||
assert_eq!(worker.worker_id.as_str(), created.worker_id.to_string());
|
||||
@@ -6750,6 +6707,87 @@ mod tests {
|
||||
runtime.stop_runtime().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_snapshot_and_worker_creation_cross_the_boundary_exactly_once() {
|
||||
let runtime = runtime_with_backend();
|
||||
let barrier = Arc::new(Barrier::new(2));
|
||||
let subscribing_runtime = runtime.clone();
|
||||
let subscribing_barrier = barrier.clone();
|
||||
let subscriber = std::thread::spawn(move || {
|
||||
subscribing_barrier.wait();
|
||||
subscribing_runtime
|
||||
.subscribe_event_selector(EventSubscriptionSelector::RuntimeWorkers)
|
||||
.unwrap()
|
||||
});
|
||||
let creating_runtime = runtime.clone();
|
||||
let creating_barrier = barrier.clone();
|
||||
let creator = std::thread::spawn(move || {
|
||||
creating_barrier.wait();
|
||||
creating_runtime
|
||||
.create_worker(task_request("concurrent-boundary"))
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
let mut subscription = subscriber.join().unwrap();
|
||||
let created = creator.join().unwrap();
|
||||
let SubscriptionSnapshot::Workers { workers } = subscription.snapshot() else {
|
||||
panic!("runtime_workers must return a Worker snapshot");
|
||||
};
|
||||
let in_snapshot = workers
|
||||
.iter()
|
||||
.any(|worker| worker.worker_id.as_str() == created.worker_id.to_string());
|
||||
let in_event = match subscription.receiver.try_recv() {
|
||||
Ok(RuntimeSubscriptionUpdate {
|
||||
payload: SubscriptionEventPayload::WorkerUpserted { worker },
|
||||
}) => worker.worker_id.as_str() == created.worker_id.to_string(),
|
||||
Ok(update) => panic!("unexpected subscription update: {update:?}"),
|
||||
Err(mpsc::error::TryRecvError::Empty) => false,
|
||||
Err(error) => panic!("unexpected subscription receive error: {error}"),
|
||||
};
|
||||
assert_ne!(
|
||||
in_snapshot, in_event,
|
||||
"the concurrent creation must appear in the snapshot or its ordered suffix, never both"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(feature = "fs-store")]
|
||||
#[test]
|
||||
fn restarted_runtime_subscription_starts_from_persisted_authoritative_snapshot() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let options = crate::fs_store::FsRuntimeStoreOptions {
|
||||
root: root.path().join("runtime"),
|
||||
runtime_id: "subscription-restart".to_string(),
|
||||
display_name: None,
|
||||
};
|
||||
let runtime = Runtime::with_fs_store_and_execution_backend(
|
||||
options.clone(),
|
||||
Arc::new(TestExecutionBackend::default()),
|
||||
)
|
||||
.unwrap();
|
||||
runtime.store_config_bundle(test_bundle()).unwrap();
|
||||
let worker = runtime
|
||||
.create_worker(task_request("persisted-subscription-worker"))
|
||||
.unwrap();
|
||||
drop(runtime);
|
||||
|
||||
let restarted = Runtime::with_fs_store_and_execution_backend(
|
||||
options,
|
||||
Arc::new(TestExecutionBackend::default()),
|
||||
)
|
||||
.unwrap();
|
||||
let subscription = restarted
|
||||
.subscribe_event_selector(EventSubscriptionSelector::RuntimeWorkers)
|
||||
.unwrap();
|
||||
let SubscriptionSnapshot::Workers { workers } = subscription.snapshot() else {
|
||||
panic!("runtime_workers must return a Worker snapshot");
|
||||
};
|
||||
assert!(workers.iter().any(|candidate| {
|
||||
candidate.worker_id.as_str() == worker.worker_id.to_string()
|
||||
&& candidate.availability
|
||||
== protocol::subscription::SubscriptionWorkerAvailability::Observed
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn worker_lifecycle_subscription_delivers_only_selected_workers() {
|
||||
let runtime = runtime_with_backend();
|
||||
|
||||
Reference in New Issue
Block a user