fix durable job notification receipts
This commit is contained in:
@@ -66,6 +66,12 @@ pub struct WorkerSubmissionAck {
|
||||
pub disposition: protocol::SubmissionDisposition,
|
||||
}
|
||||
|
||||
/// Evidence that a notification request reached the durable Worker session boundary.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct WorkerNotificationAck {
|
||||
pub notification_request_id: String,
|
||||
}
|
||||
|
||||
/// Typed execution result class. Results are transient operation outcomes and
|
||||
/// are not persisted as Worker lifecycle authority.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
@@ -78,6 +84,8 @@ pub struct WorkerExecutionResult {
|
||||
pub message: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub submission: Option<WorkerSubmissionAck>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub notification: Option<WorkerNotificationAck>,
|
||||
}
|
||||
|
||||
/// Backend result class for a Worker execution operation.
|
||||
@@ -99,6 +107,7 @@ impl WorkerExecutionResult {
|
||||
worker_state: None,
|
||||
message: None,
|
||||
submission: None,
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,6 +132,23 @@ impl WorkerExecutionResult {
|
||||
submission_id: submission_id.into(),
|
||||
disposition,
|
||||
}),
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn accepted_notification(
|
||||
operation: WorkerExecutionOperation,
|
||||
notification_request_id: impl Into<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
operation,
|
||||
outcome: WorkerExecutionOutcome::Accepted,
|
||||
worker_state: None,
|
||||
message: None,
|
||||
submission: None,
|
||||
notification: Some(WorkerNotificationAck {
|
||||
notification_request_id: notification_request_id.into(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,6 +159,7 @@ impl WorkerExecutionResult {
|
||||
worker_state: None,
|
||||
message: Some(message.into()),
|
||||
submission: None,
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -143,6 +170,7 @@ impl WorkerExecutionResult {
|
||||
worker_state: None,
|
||||
message: Some(message.into()),
|
||||
submission: None,
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -153,6 +181,7 @@ impl WorkerExecutionResult {
|
||||
worker_state: None,
|
||||
message: Some(message.into()),
|
||||
submission: None,
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -163,6 +192,7 @@ impl WorkerExecutionResult {
|
||||
worker_state: None,
|
||||
message: Some(message.into()),
|
||||
submission: None,
|
||||
notification: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -94,6 +94,9 @@ pub struct WorkerInteractionAck {
|
||||
pub worker_ref: WorkerRef,
|
||||
pub status: WorkerStatus,
|
||||
/// Present for User Submit and absent for non-Submit interactions.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub submission: Option<crate::execution::WorkerSubmissionAck>,
|
||||
/// Present only after a Notify request reaches the Worker's durable receipt boundary.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub notification: Option<crate::execution::WorkerNotificationAck>,
|
||||
}
|
||||
|
||||
@@ -2104,6 +2104,12 @@ impl Runtime {
|
||||
.clone()
|
||||
.expect("User input request id must be assigned before dispatch")
|
||||
});
|
||||
let expected_notification_id = (input.kind == WorkerInputKind::Notify).then(|| {
|
||||
input
|
||||
.submission_request_id
|
||||
.clone()
|
||||
.expect("Notify request id must be assigned before dispatch")
|
||||
});
|
||||
self.ensure_worker_execution(worker_ref)?;
|
||||
let (backend, handle) = {
|
||||
let state = self.lock()?;
|
||||
@@ -2160,7 +2166,28 @@ impl Runtime {
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(expected_notification_id) = expected_notification_id
|
||||
&& dispatch_result
|
||||
.notification
|
||||
.as_ref()
|
||||
.is_none_or(|ack| ack.notification_request_id != expected_notification_id)
|
||||
{
|
||||
let result = WorkerExecutionResult::rejected(
|
||||
WorkerExecutionOperation::Input,
|
||||
"execution backend did not acknowledge the durable Runtime notification request id",
|
||||
);
|
||||
self.record_execution_result(worker_ref, result.clone())?;
|
||||
return Err(RuntimeError::WorkerExecutionRejected {
|
||||
worker_id: worker_ref.worker_id.clone(),
|
||||
operation: result.operation,
|
||||
outcome: result.outcome,
|
||||
message: result.message_or_default(),
|
||||
result,
|
||||
});
|
||||
}
|
||||
|
||||
let submission = dispatch_result.submission.clone();
|
||||
let notification = dispatch_result.notification.clone();
|
||||
let mut state = self.lock()?;
|
||||
state.ensure_running()?;
|
||||
let worker = state.worker_mut(worker_ref)?;
|
||||
@@ -2180,6 +2207,7 @@ impl Runtime {
|
||||
worker_ref: worker_ref.clone(),
|
||||
status,
|
||||
submission,
|
||||
notification,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -6451,7 +6479,8 @@ mod tests {
|
||||
_handle: &WorkerExecutionHandle,
|
||||
input: WorkerInput,
|
||||
) -> WorkerExecutionResult {
|
||||
let submission_id = input.submission_request_id.clone();
|
||||
let request_id = input.submission_request_id.clone();
|
||||
let input_kind = input.kind.clone();
|
||||
self.dispatched_inputs.lock().unwrap().push(input);
|
||||
let mut result = self
|
||||
.dispatch_result
|
||||
@@ -6459,20 +6488,28 @@ mod tests {
|
||||
.unwrap()
|
||||
.clone()
|
||||
.unwrap_or_else(|| {
|
||||
WorkerExecutionResult::accepted_submission(
|
||||
WorkerExecutionOperation::Input,
|
||||
"request-test",
|
||||
"test-submission",
|
||||
protocol::SubmissionDisposition::Started,
|
||||
)
|
||||
if input_kind == WorkerInputKind::Notify {
|
||||
WorkerExecutionResult::accepted_notification(
|
||||
WorkerExecutionOperation::Input,
|
||||
request_id
|
||||
.clone()
|
||||
.expect("Runtime assigns Notify request identity before dispatch"),
|
||||
)
|
||||
} else {
|
||||
WorkerExecutionResult::accepted_submission(
|
||||
WorkerExecutionOperation::Input,
|
||||
"request-test",
|
||||
"test-submission",
|
||||
protocol::SubmissionDisposition::Started,
|
||||
)
|
||||
}
|
||||
});
|
||||
if !self
|
||||
.preserve_submission_acknowledgement_id
|
||||
.load(Ordering::SeqCst)
|
||||
&& let (Some(ack), Some(submission_id)) =
|
||||
(result.submission.as_mut(), submission_id)
|
||||
&& let (Some(ack), Some(request_id)) = (result.submission.as_mut(), request_id)
|
||||
{
|
||||
ack.submission_request_id = submission_id;
|
||||
ack.submission_request_id = request_id;
|
||||
}
|
||||
result
|
||||
}
|
||||
@@ -7911,6 +7948,29 @@ mod tests {
|
||||
assert!(runtime.list_workers().unwrap().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn notify_rejects_backend_enqueue_without_durable_worker_receipt() {
|
||||
let (runtime, backend) = runtime_and_backend();
|
||||
backend.set_dispatch_result(WorkerExecutionResult::accepted(
|
||||
WorkerExecutionOperation::Input,
|
||||
));
|
||||
let detail = runtime
|
||||
.create_worker(task_request("notify receipt"))
|
||||
.unwrap();
|
||||
let mut input = WorkerInput::notify("do not acknowledge enqueue alone");
|
||||
input.submission_request_id = Some("notification-request".to_string());
|
||||
|
||||
let error = runtime.send_input(&detail.worker_ref, input).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
error,
|
||||
RuntimeError::WorkerExecutionRejected {
|
||||
outcome: crate::execution::WorkerExecutionOutcome::Rejected,
|
||||
..
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn connected_backend_busy_dispatch_is_typed_and_not_transcribed() {
|
||||
let (runtime, backend) = runtime_and_backend();
|
||||
|
||||
@@ -45,6 +45,11 @@ use protocol::{Event, Method, Segment, WorkerCommandEnvelope};
|
||||
|
||||
static NEXT_INTERNAL_COMMAND_ID: AtomicU64 = AtomicU64::new(1);
|
||||
|
||||
enum NotificationAcceptance {
|
||||
Accepted,
|
||||
Rejected(String),
|
||||
}
|
||||
|
||||
fn rollback_materialized_spawn_workdirs(
|
||||
materializer: &dyn WorkingDirectoryMaterializer,
|
||||
bindings: &BTreeMap<WorkdirAttachmentAlias, WorkingDirectoryBinding>,
|
||||
@@ -1883,6 +1888,75 @@ where
|
||||
.unwrap_or_else(|message| WorkerExecutionResult::errored(operation, message))
|
||||
}
|
||||
|
||||
fn send_notification_and_wait_for_acceptance(
|
||||
&self,
|
||||
operation: WorkerExecutionOperation,
|
||||
worker: WorkerHandle,
|
||||
method: Method,
|
||||
notification_request_id: String,
|
||||
) -> WorkerExecutionResult {
|
||||
let request_id = notification_request_id.clone();
|
||||
let result = self.run_cancellable_on_adapter_runtime(USER_INPUT_TASK_TIMEOUT, async move {
|
||||
// Subscribe before enqueueing so a fast durable acceptance cannot
|
||||
// race the Runtime acknowledgement.
|
||||
let mut events = worker.subscribe();
|
||||
worker
|
||||
.send(method)
|
||||
.await
|
||||
.map_err(|err| format!("failed to send Worker method: {err}"))?;
|
||||
|
||||
tokio::time::timeout(USER_INPUT_COMMIT_TIMEOUT, async move {
|
||||
loop {
|
||||
match events.recv().await {
|
||||
Ok(Event::NotificationAccepted {
|
||||
notification_request_id,
|
||||
}) if notification_request_id == request_id => {
|
||||
return Ok(NotificationAcceptance::Accepted);
|
||||
}
|
||||
Ok(Event::NotificationRejected {
|
||||
notification_request_id,
|
||||
message,
|
||||
}) if notification_request_id == request_id => {
|
||||
return Ok(NotificationAcceptance::Rejected(message));
|
||||
}
|
||||
Ok(Event::Shutdown) => {
|
||||
return Err(
|
||||
"worker shut down before notification was durably accepted"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => {
|
||||
return Err(format!(
|
||||
"worker notification acknowledgement lagged by {skipped} protocol event(s)"
|
||||
));
|
||||
}
|
||||
Err(tokio::sync::broadcast::error::RecvError::Closed) => {
|
||||
return Err(
|
||||
"worker event stream closed before notification was durably accepted"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
.await
|
||||
.map_err(|_| {
|
||||
"timed out waiting for durable Worker notification acceptance".to_string()
|
||||
})?
|
||||
});
|
||||
|
||||
match result {
|
||||
Ok(NotificationAcceptance::Accepted) => {
|
||||
WorkerExecutionResult::accepted_notification(operation, notification_request_id)
|
||||
}
|
||||
Ok(NotificationAcceptance::Rejected(message)) => {
|
||||
WorkerExecutionResult::rejected(operation, message)
|
||||
}
|
||||
Err(message) => WorkerExecutionResult::errored(operation, message),
|
||||
}
|
||||
}
|
||||
|
||||
fn retain_uncertain_unconnected_controller(
|
||||
&self,
|
||||
worker_ref: &crate::identity::WorkerRef,
|
||||
@@ -2879,16 +2953,17 @@ where
|
||||
let notification_request_id = input
|
||||
.submission_request_id
|
||||
.unwrap_or_else(protocol::new_submission_request_id);
|
||||
return self.send_method(
|
||||
return self.send_notification_and_wait_for_acceptance(
|
||||
WorkerExecutionOperation::Input,
|
||||
worker,
|
||||
Method::NotifyTracked {
|
||||
notification_request_id: notification_request_id.clone(),
|
||||
message: input.content,
|
||||
source: protocol::AuthenticatedInputSource::Backend {
|
||||
operation_id: notification_request_id,
|
||||
operation_id: notification_request_id.clone(),
|
||||
},
|
||||
},
|
||||
notification_request_id,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5108,7 +5183,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn running_worker_accepts_notify_without_a_submit_receipt() {
|
||||
fn running_worker_waits_for_durable_notify_receipt() {
|
||||
let client = MockClient::sequential(vec![MockResponse::Hang(vec![])]);
|
||||
let runtime_base = tempfile::tempdir().unwrap();
|
||||
let cwd = tempfile::tempdir().unwrap();
|
||||
@@ -5143,6 +5218,13 @@ mod tests {
|
||||
.expect("Running Worker must accept Notify without a Submit receipt");
|
||||
|
||||
assert!(acknowledgement.submission.is_none());
|
||||
assert_eq!(
|
||||
acknowledgement
|
||||
.notification
|
||||
.as_ref()
|
||||
.map(|ack| ack.notification_request_id.as_str()),
|
||||
Some("notification-request")
|
||||
);
|
||||
wait_for_adapter_state(&backend, &detail.worker_ref, WorkerStatus::Running);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user