Skip to content

Commit cd7ac97

Browse files
committed
refactor(workflow,grpc,webapi): Record auto-upgrade outcome in history
Replace the success-only `ComponentUpgraded` event with `ComponentUpgradeFinished` so both successful and failed auto-upgrades are recorded in the execution log. Failed upgrades mark the new digest as incompatible through the log instead of updating t_state directly. Tighten `Unlocked` so it only releases a locked execution back to `PendingAt`, while keeping the previous `backoff_expires_at` wire shape for JSON/proto compatibility. Auto-upgrade appends `Unlocked` only when replay stops during a stub-response flush and would otherwise leave the execution locked, leading to a timeout. This fixes the case where auto-upgrade unconditionally moved executions to `PendingAt` (via `ComponentUpgraded`, allowing a blocked workflow to be picked up immediately and panic on the next execution run if the response had not arrived yet. Add tests for blocked upgrades, failed upgrades, and the stub-write path that still needs Unlocked.
1 parent 4076946 commit cd7ac97

16 files changed

Lines changed: 1120 additions & 542 deletions

File tree

assets/schemas/db.json

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -147,24 +147,11 @@
147147
"additionalProperties": false
148148
},
149149
{
150-
"description": "Returns execution to [`PendingState::PendingAt`] state at the specified time.\nThis can happen when:\n- executor is running out of resources like [`WorkerError::LimitReached`]\n- executor is being closed (shutdown or hot redeploy requested)\n- activity is paused\n- workflow made progress but then its lock expired\n- workflow attempted to auto-upgrade an execution and failed, recording its digest in `incompatible_digest`",
150+
"description": "Releases a lock.\n\nState transition semantics:\n- [`PendingState::Locked`] becomes [`PendingState::PendingAt`] at\n [`Unlocked::backoff_expires_at`]. The field name is kept for persisted JSON and gRPC\n compatibility, but it is the next pending instant for every unlock reason.\n- [`PendingState::PendingAt`], [`PendingState::BlockedByJoinSet`],\n [`PendingState::Paused`], and [`PendingState::Finished`] reject this event.",
151151
"type": "object",
152152
"properties": {
153153
"unlocked": {
154-
"type": "object",
155-
"properties": {
156-
"backoff_expires_at": {
157-
"type": "string",
158-
"format": "date-time"
159-
},
160-
"reason": {
161-
"$ref": "#/$defs/UnlockedReason"
162-
}
163-
},
164-
"required": [
165-
"backoff_expires_at",
166-
"reason"
167-
]
154+
"$ref": "#/$defs/Unlocked"
168155
}
169156
},
170157
"required": [
@@ -173,9 +160,10 @@
173160
"additionalProperties": false
174161
},
175162
{
163+
"description": "Does not change `PendingState`.",
176164
"type": "object",
177165
"properties": {
178-
"component_upgraded": {
166+
"component_upgrade_finished": {
179167
"type": "object",
180168
"properties": {
181169
"component_digest": {
@@ -184,19 +172,19 @@
184172
"deployment_id": {
185173
"type": "string"
186174
},
187-
"reason": {
188-
"$ref": "#/$defs/ComponentUpgradeReason"
175+
"outcome": {
176+
"$ref": "#/$defs/ComponentUpgradeOutcome"
189177
}
190178
},
191179
"required": [
192180
"component_digest",
193181
"deployment_id",
194-
"reason"
182+
"outcome"
195183
]
196184
}
197185
},
198186
"required": [
199-
"component_upgraded"
187+
"component_upgrade_finished"
200188
],
201189
"additionalProperties": false
202190
},
@@ -410,17 +398,34 @@
410398
"nanos"
411399
]
412400
},
413-
"UnlockedReason": {
401+
"Unlocked": {
402+
"type": "object",
403+
"properties": {
404+
"backoff_expires_at": {
405+
"description": "Instant used when releasing a currently locked execution back to\n[`PendingState::PendingAt`]. This field keeps the released JSON and gRPC name.",
406+
"type": "string",
407+
"format": "date-time"
408+
},
409+
"reason": {
410+
"type": "string"
411+
}
412+
},
413+
"required": [
414+
"backoff_expires_at",
415+
"reason"
416+
]
417+
},
418+
"ComponentUpgradeOutcome": {
414419
"oneOf": [
415420
{
416421
"type": "object",
417422
"properties": {
418423
"reason": {
419-
"type": "string"
424+
"$ref": "#/$defs/ComponentUpgradeReason"
420425
},
421426
"type": {
422427
"type": "string",
423-
"const": "other"
428+
"const": "success"
424429
}
425430
},
426431
"required": [
@@ -431,26 +436,23 @@
431436
{
432437
"type": "object",
433438
"properties": {
434-
"target_digest": {
435-
"type": "string"
436-
},
437439
"reason": {
438440
"type": "string"
439441
},
440442
"type": {
441443
"type": "string",
442-
"const": "auto_upgrade_failed"
444+
"const": "failed"
443445
}
444446
},
445447
"required": [
446448
"type",
447-
"target_digest",
448449
"reason"
449450
]
450451
}
451452
]
452453
},
453454
"ComponentUpgradeReason": {
455+
"description": "Reason for auditing only",
454456
"oneOf": [
455457
{
456458
"type": "object",

crates/concepts/src/storage.rs

Lines changed: 43 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -360,26 +360,24 @@ pub enum ExecutionRequest {
360360
scheduled_by: Option<ExecutionId>,
361361
},
362362
Locked(Locked),
363-
/// Returns execution to [`PendingState::PendingAt`] state at the specified time.
364-
/// This can happen when:
365-
/// - executor is running out of resources like [`WorkerError::LimitReached`]
366-
/// - executor is being closed (shutdown or hot redeploy requested)
367-
/// - activity is paused
368-
/// - workflow made progress but then its lock expired
369-
/// - workflow attempted to auto-upgrade an execution and failed, recording its digest in `incompatible_digest`
370-
#[display("Unlocked(`{backoff_expires_at}`)")]
371-
Unlocked {
372-
backoff_expires_at: DateTime<Utc>,
373-
#[cfg_attr(any(test, feature = "test"), arbitrary(value = UnlockedReason::Other { reason: StrVariant::Static("reason") }))]
374-
reason: UnlockedReason,
375-
},
376-
#[display("ComponentUpgraded({component_digest})")]
377-
ComponentUpgraded {
363+
/// Releases a lock.
364+
///
365+
/// State transition semantics:
366+
/// - [`PendingState::Locked`] becomes [`PendingState::PendingAt`] at
367+
/// [`Unlocked::backoff_expires_at`]. The field name is kept for persisted JSON and gRPC
368+
/// compatibility, but it is the next pending instant for every unlock reason.
369+
/// - [`PendingState::PendingAt`], [`PendingState::BlockedByJoinSet`],
370+
/// [`PendingState::Paused`], and [`PendingState::Finished`] reject this event.
371+
#[display("Unlocked({_0})")]
372+
Unlocked(Unlocked),
373+
/// Does not change `PendingState`.
374+
#[display("ComponentUpgradeFinished({component_digest})")]
375+
ComponentUpgradeFinished {
378376
#[cfg_attr(any(test, feature = "test"), arbitrary(value = ComponentId::dummy_activity().component_digest))]
379377
component_digest: ComponentDigest,
380378
#[cfg_attr(any(test, feature = "test"), arbitrary(value = DeploymentId::from_parts(0, 0)))]
381379
deployment_id: DeploymentId,
382-
reason: ComponentUpgradeReason,
380+
outcome: ComponentUpgradeOutcome,
383381
},
384382
// Created by the executor holding the lock.
385383
// After expiry interpreted as pending.
@@ -419,24 +417,7 @@ pub enum ExecutionRequest {
419417
Unpaused,
420418
}
421419

422-
#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display, Serialize, schemars::JsonSchema)]
423-
#[cfg_attr(any(test, feature = "test"), derive(arbitrary::Arbitrary))]
424-
#[serde(tag = "type", rename_all = "snake_case")]
425-
pub enum UnlockedReason {
426-
#[display("{reason}")]
427-
Other {
428-
#[cfg_attr(any(test, feature = "test"), arbitrary(value = StrVariant::Static("reason")))]
429-
reason: StrVariant,
430-
},
431-
#[display("auto-upgrade failed: {reason}")]
432-
AutoUpgradeFailed {
433-
#[cfg_attr(any(test, feature = "test"), arbitrary(value = ComponentId::dummy_activity().component_digest))]
434-
target_digest: ComponentDigest,
435-
#[cfg_attr(any(test, feature = "test"), arbitrary(value = StrVariant::Static("reason")))]
436-
reason: StrVariant,
437-
},
438-
}
439-
420+
/// Reason for auditing only
440421
#[derive(
441422
Clone, Debug, PartialEq, Eq, derive_more::Display, Serialize, Deserialize, schemars::JsonSchema,
442423
)]
@@ -449,91 +430,32 @@ pub enum ComponentUpgradeReason {
449430
Manual { force: bool },
450431
}
451432

452-
impl<'de> Deserialize<'de> for UnlockedReason {
453-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
454-
where
455-
D: serde::Deserializer<'de>,
456-
{
457-
unlocked_reason_compat::deserialize(deserializer)
458-
}
459-
}
460-
461-
mod unlocked_reason_compat {
462-
use super::{ComponentDigest, StrVariant, UnlockedReason};
463-
use serde::Deserialize;
464-
465-
// TODO: Delete this compatibility module with the next DB-breaking release.
466-
pub(super) fn deserialize<'de, D>(deserializer: D) -> Result<UnlockedReason, D::Error>
467-
where
468-
D: serde::Deserializer<'de>,
469-
{
470-
Compat::deserialize(deserializer).map(Into::into)
471-
}
472-
473-
#[derive(Deserialize)]
474-
#[serde(untagged)]
475-
enum Compat {
476-
Current(Current),
477-
Legacy(StrVariant),
478-
}
479-
480-
#[derive(Deserialize)]
481-
#[serde(tag = "type", rename_all = "snake_case")]
482-
enum Current {
483-
Other {
484-
reason: StrVariant,
485-
},
486-
AutoUpgradeFailed {
487-
target_digest: ComponentDigest,
488-
reason: StrVariant,
489-
},
490-
}
491-
492-
impl From<Compat> for UnlockedReason {
493-
fn from(value: Compat) -> Self {
494-
match value {
495-
Compat::Current(current) => current.into(),
496-
Compat::Legacy(reason) => Self::Other { reason },
497-
}
498-
}
499-
}
500-
501-
impl From<Current> for UnlockedReason {
502-
fn from(value: Current) -> Self {
503-
match value {
504-
Current::Other { reason } => Self::Other { reason },
505-
Current::AutoUpgradeFailed {
506-
target_digest,
507-
reason,
508-
} => Self::AutoUpgradeFailed {
509-
target_digest,
510-
reason,
511-
},
512-
}
513-
}
514-
}
515-
}
516-
517-
impl From<StrVariant> for UnlockedReason {
518-
fn from(value: StrVariant) -> Self {
519-
Self::Other { reason: value }
520-
}
521-
}
522-
523-
impl From<String> for UnlockedReason {
524-
fn from(value: String) -> Self {
525-
Self::Other {
526-
reason: StrVariant::from(value),
527-
}
528-
}
433+
#[derive(
434+
Clone, Debug, PartialEq, Eq, derive_more::Display, Serialize, Deserialize, schemars::JsonSchema,
435+
)]
436+
#[cfg_attr(any(test, feature = "test"), derive(arbitrary::Arbitrary))]
437+
#[display("{reason}, pending at {backoff_expires_at}")]
438+
pub struct Unlocked {
439+
/// Instant used when releasing a currently locked execution back to
440+
/// [`PendingState::PendingAt`]. This field keeps the released JSON and gRPC name.
441+
pub backoff_expires_at: DateTime<Utc>,
442+
#[cfg_attr(any(test, feature = "test"), arbitrary(value = StrVariant::Static("reason")))]
443+
pub reason: StrVariant,
529444
}
530445

531-
impl From<&'static str> for UnlockedReason {
532-
fn from(value: &'static str) -> Self {
533-
Self::Other {
534-
reason: StrVariant::Static(value),
535-
}
536-
}
446+
#[derive(
447+
Clone, Debug, PartialEq, Eq, derive_more::Display, Serialize, Deserialize, schemars::JsonSchema,
448+
)]
449+
#[cfg_attr(any(test, feature = "test"), derive(arbitrary::Arbitrary))]
450+
#[serde(tag = "type", rename_all = "snake_case")]
451+
pub enum ComponentUpgradeOutcome {
452+
#[display("success({reason})")]
453+
Success { reason: ComponentUpgradeReason },
454+
#[display("failed: {reason}")]
455+
Failed {
456+
#[cfg_attr(any(test, feature = "test"), arbitrary(value = StrVariant::Static("reason")))]
457+
reason: StrVariant,
458+
},
537459
}
538460

539461
impl ExecutionRequest {
@@ -551,8 +473,8 @@ impl ExecutionRequest {
551473
match self {
552474
ExecutionRequest::Created { .. } => "created",
553475
ExecutionRequest::Locked(_) => "locked",
554-
ExecutionRequest::Unlocked { .. } => "unlocked",
555-
ExecutionRequest::ComponentUpgraded { .. } => "component_upgraded",
476+
ExecutionRequest::Unlocked(_) => "unlocked",
477+
ExecutionRequest::ComponentUpgradeFinished { .. } => "component_upgrade_finished",
556478
ExecutionRequest::TemporarilyFailed { .. } => "temporarily_failed",
557479
ExecutionRequest::TemporarilyTimedOut { .. } => "temporarily_timed_out",
558480
ExecutionRequest::Finished { .. } => "finished",
@@ -1575,8 +1497,8 @@ pub trait DbExternalApi: DbConnection {
15751497
) -> Result<Vec<DeploymentRecord>, DbErrorRead>;
15761498

15771499
/// Pause an execution. Only pending executions can be paused.
1578-
/// If the execution is currently locked, implementations must release the lock first
1579-
/// and then record the paused state.
1500+
/// If the execution is an activity and is currently in `PendingState::Locked`, implementations must
1501+
/// append `ExecutionRequest::Unlocked` to avoid affecting failed attempt count, before appending `ExecutionRequest::Paused`.
15801502
async fn pause_execution(
15811503
&self,
15821504
execution_id: &ExecutionId,
@@ -2693,10 +2615,8 @@ mod tests {
26932615
use super::PendingStateFinished;
26942616
use super::PendingStateFinishedError;
26952617
use super::PendingStateFinishedResultKind;
2696-
use super::UnlockedReason;
26972618
use crate::ExecutionFailureKind;
26982619
use crate::JoinSetId;
2699-
use crate::StrVariant;
27002620
use crate::SupportedFunctionReturnValue;
27012621
use chrono::DateTime;
27022622
use chrono::Datelike;
@@ -2738,38 +2658,6 @@ mod tests {
27382658
assert_eq!(expected, actual);
27392659
}
27402660

2741-
#[test]
2742-
fn unlocked_reason_should_deserialize_legacy_string() {
2743-
let actual: UnlockedReason = serde_json::from_str(r#""executor closing""#).unwrap();
2744-
assert_eq!(
2745-
UnlockedReason::Other {
2746-
reason: StrVariant::Static("executor closing"),
2747-
},
2748-
actual
2749-
);
2750-
}
2751-
2752-
#[test]
2753-
fn unlocked_reason_other_should_serialize_current_tagged_shape() {
2754-
let actual = serde_json::to_string(&UnlockedReason::Other {
2755-
reason: StrVariant::Static("executor closing"),
2756-
})
2757-
.unwrap();
2758-
assert_eq!(r#"{"type":"other","reason":"executor closing"}"#, actual);
2759-
}
2760-
2761-
#[test]
2762-
fn unlocked_reason_should_deserialize_current_tagged_shape() {
2763-
let actual: UnlockedReason =
2764-
serde_json::from_str(r#"{"type":"other","reason":"executor closing"}"#).unwrap();
2765-
assert_eq!(
2766-
UnlockedReason::Other {
2767-
reason: StrVariant::Static("executor closing"),
2768-
},
2769-
actual
2770-
);
2771-
}
2772-
27732661
#[test]
27742662
fn join_set_deser_with_result_ok_option_none_should_work() {
27752663
let expected = SupportedFunctionReturnValue::Ok(Some(WastValWithType {

0 commit comments

Comments
 (0)