diff --git a/crates/protos/protos/api_upstream/temporal/api/enums/v1/stream.proto b/crates/protos/protos/api_upstream/temporal/api/enums/v1/stream.proto index 1a258f97d..98104ca9e 100644 --- a/crates/protos/protos/api_upstream/temporal/api/enums/v1/stream.proto +++ b/crates/protos/protos/api_upstream/temporal/api/enums/v1/stream.proto @@ -20,6 +20,16 @@ enum StreamStatus { STREAM_STATUS_EXPIRED = 3; } +// Experimental. Might change in the future. +// How a Stream closed. CLOSED comes from a Producer that called close(). Native Streams add their +// own close statuses later. +// (-- api-linter: core::0216::synonyms=disabled +// aip.dev/not-precedent: Named consistently with WorkflowExecutionStatus. --) +enum StreamCloseStatus { + STREAM_CLOSE_STATUS_UNSPECIFIED = 0; + STREAM_CLOSE_STATUS_CLOSED = 1; +} + // Experimental. Might change in the future. // Where a read starts when it has no from_offset. enum StreamReadStart { @@ -49,16 +59,14 @@ enum StreamErrorReason { STREAM_ERROR_REASON_EXPIRED = 2; // The Stream is closed. FAILED_PRECONDITION. STREAM_ERROR_REASON_CLOSED = 3; - // The Producer's epoch is lower than the newest one, its Workflow Task was withdrawn, or a - // Workflow retry withdrew its run. FAILED_PRECONDITION. + // The Producer's epoch is lower than the newest one. FAILED_PRECONDITION. STREAM_ERROR_REASON_FENCED = 4; // The batch has a lower `sequence` than the last one of its epoch. FAILED_PRECONDITION. STREAM_ERROR_REASON_SEQUENCE_TOO_LOW = 5; // The batch has the `sequence` of the last one of its epoch, and other content. // FAILED_PRECONDITION. STREAM_ERROR_REASON_SEQ_DIVERGENT = 6; - // A create gives another owner than the Stream has, or the owner doesn't allow this close. - // FAILED_PRECONDITION. + // The caller may not create or close this Stream. FAILED_PRECONDITION. STREAM_ERROR_REASON_NOT_OWNER = 7; // A newer format version wrote the Stream. FAILED_PRECONDITION. STREAM_ERROR_REASON_FORMAT_TOO_NEW = 8; diff --git a/crates/protos/protos/api_upstream/temporal/api/stream/v1/message.proto b/crates/protos/protos/api_upstream/temporal/api/stream/v1/message.proto index 7385028a1..fcd2d8560 100644 --- a/crates/protos/protos/api_upstream/temporal/api/stream/v1/message.proto +++ b/crates/protos/protos/api_upstream/temporal/api/stream/v1/message.proto @@ -18,11 +18,6 @@ import "temporal/api/enums/v1/stream.proto"; // Identifies one Stream. The id is opaque to callers. message StreamRef { string id = 1; - // Unset for a standalone Stream. For a Stream bound to a Workflow, business_id is the Workflow - // ID and run_id is the first execution run id of its run chain. For a Standalone Activity, - // business_id is the activity ID and run_id is its run id. A create and a close of a - // Workflow-owned Stream must carry its owner. - temporal.api.common.v1.Execution owner = 2; // The name of the backend that holds the Stream, as the Worker and client config register // it, so every process routes a ref it receives. Empty is the default backend. The Store // never reads it. @@ -65,7 +60,7 @@ message ClientEpoch { } // Experimental. Might change in the future. -// A Workflow run Producer. It writes in its Workflow Task. A higher epoch withdraws the Records of +// A Workflow run Producer. It writes in its Workflow Task. A higher epoch supersedes the Records of // the run's Workflow Tasks that never completed, with those of their Local Activities. message WorkflowProducer { // The first execution run id of the run chain. @@ -74,7 +69,7 @@ message WorkflowProducer { } // Experimental. Might change in the future. -// A Local Activity Producer. A higher epoch withdraws the Producer's lower epochs. +// A Local Activity Producer. A higher epoch supersedes the Producer's lower epochs. message LocalActivityProducer { string run_id = 1; LocalActivityEpoch epoch = 2; @@ -82,7 +77,7 @@ message LocalActivityProducer { // Experimental. Might change in the future. // An Activity Producer, for an Activity of a Workflow or a Standalone Activity. A higher epoch -// withdraws the Producer's lower epochs. +// supersedes the Producer's lower epochs. message ActivityProducer { string run_id = 1; ActivityEpoch epoch = 2; @@ -97,7 +92,7 @@ message ClientProducer { // Experimental. Might change in the future. // The code that wrote a batch. Its id stays the same across attempts, and the kind's case orders -// its epochs and picks which earlier Records a higher epoch withdraws. A run-based kind carries its +// its epochs and picks which earlier Records a higher epoch supersedes. A run-based kind carries its // run, so a client can't. message Producer { // Stable across attempts, such as `wf:` for the Workflow Producer of a run. @@ -113,30 +108,11 @@ message Producer { } // Experimental. Might change in the future. -// The Producer of a write, and which earlier Records the write withdraws. An append, a close and -// a writer carry the same fields, so the Store orders and withdraws each write the same way. +// The Producer of a write. An append and a close carry the same writer, so the Store orders each +// write the same way. message StreamWriter { - // Required on a write. Its Producer's kind picks the withdrawal rule. + // Required on a write. Producer producer = 1; - // Set only by a Workflow or Local Activity write, since only a run withdraws orphaned Local - // Activity Records or a previous run. Unset for an Activity or client write. - RunWithdrawal withdrawal = 2; -} - -// Experimental. Might change in the future. -// What a Workflow or Local Activity write withdraws from its run. One write can carry both the task -// facts and a run to withdraw, so these are fields of one message and not a oneof. -message RunWithdrawal { - // The run's last completed Workflow Task, from a Local Activity's start. Its started_event_id - // is 0 before the first. The write withdraws the run's Local Activity Records whose Workflow - // Task never completed. - WorkflowTaskEpoch previous_completed_task = 1; - // The started event ids of the run's failed or timed out Workflow Tasks. The write withdraws - // the run's Local Activity Records of these tasks. - repeated int64 failed_tasks = 2; - // Retry run only. The previous run, on the retry run's first write. Every Record whose Producer - // carries it is withdrawn. - string withdraw_run = 3; } // Experimental. Might change in the future. @@ -145,15 +121,6 @@ message AppendRecord { temporal.api.common.v1.Payload body = 1; } -// Experimental. Might change in the future. -// Which earlier Records a retried Producer withdraws. The Store keeps these facts for the first -// batch of a higher epoch, or for a retry run's first write, and a read builds a Superseded item -// from them. A reader drops the earlier Records that the facts match. -message StreamSuperseded { - // The writer of the batch that withdraws, with its withdrawal facts. - StreamWriter writer = 1; -} - // Experimental. Might change in the future. // One Record that a Producer appended. message StreamData { @@ -161,19 +128,27 @@ message StreamData { temporal.api.common.v1.Payload body = 2; } +// Experimental. Might change in the future. +// The close of a Stream. A Stream has at most one, and a read returns it last. +message StreamClose { + // The Producer that closed the Stream. + Producer producer = 1; + reserved 2, 3; + reserved "result", "failure"; + // Why the Stream closed. + temporal.api.enums.v1.StreamCloseStatus status = 4; +} + // Experimental. Might change in the future. // An offset is ordered and unique in one Stream, and it is sparse. Offsets have gaps, so they // don't count Records. Zero is never an offset. A read gives its Stream, so the read checks the // offset against that Stream. message StreamRecord { int64 offset = 1; - // A read item is a Record or a Superseded item. A close writes no item. + // A read item is a Record or the close. oneof item { StreamData data = 2; - // The Store keeps only the withdrawal facts of the first batch of a higher epoch. A read - // builds this item from them. It withdraws the earlier Records that the facts match, and a - // reader drops them. - StreamSuperseded superseded = 3; + StreamClose close = 4; } } diff --git a/crates/protos/protos/api_upstream/temporal/api/streamservice/v1/request_response.proto b/crates/protos/protos/api_upstream/temporal/api/streamservice/v1/request_response.proto index 463f91194..e9296dbab 100644 --- a/crates/protos/protos/api_upstream/temporal/api/streamservice/v1/request_response.proto +++ b/crates/protos/protos/api_upstream/temporal/api/streamservice/v1/request_response.proto @@ -20,31 +20,19 @@ import "temporal/api/stream/v1/message.proto"; // Experimental. Might change in the future. // Creates the Stream. When the Stream exists, FAIL fails with ALREADY_EXISTS, and USE_EXISTING // gets it with created false and its own Retention. A retry of the create that made the Stream, -// with the same request_id, gets created true. Each policy fails with CLOSED on a closed Stream, -// EXPIRED on an expired one, and NOT_OWNER on one with another owner. +// with the same request_id, gets created true. Each policy fails with CLOSED on a closed Stream +// and EXPIRED on an expired one. // (-- api-linter: core::0133=disabled // aip.dev/not-precedent: The request identifies a Stream by namespace and ref. --) message CreateStreamRequest { string namespace = 1; - // The ref of the Stream to create. Its id is the Stream id, and is never empty. A bound - // Stream's ref has the owner's run chain as its owner. + // The ref of the Stream to create. Its id is the Stream id, and is never empty. temporal.api.stream.v1.StreamRef stream_ref = 2; // The age after which a Record is dropped. Empty is the Store default. google.protobuf.Duration retention = 3; - // Workflow only. The previous run on a retry run's first create, to withdraw its Records. - string withdraw_run = 4; - // The Workflow run that owns a bound Stream, with its run_id and a Workflow epoch. It needs a - // ref whose owner is the run's chain. Unset for a standalone Stream. - temporal.api.stream.v1.Producer owner = 5; - // Only read by the server. When set, the server closes the Stream when the owner's run chain - // ends, also on a terminate or a timeout. It needs owner, and a ref with a Workflow owner. - // (-- api-linter: core::0140::prepositions=disabled - // aip.dev/not-precedent: "with" ties the close to the owner's run chain. --) - bool close_with_run = 6; // Only read by the server, for a Stream bound to a Standalone Activity. The server checks that // the token is the Activity's current attempt, so a caller can't bind a Stream to a Standalone - // Activity that it isn't. The server knows the run of a Workflow's call, and a standalone - // Stream has no owner, so neither needs a token. + // Activity that it isn't. bytes task_token = 7; // What the create does when the Stream exists. Unspecified is FAIL. temporal.api.enums.v1.StreamIdConflictPolicy id_conflict_policy = 8; @@ -56,7 +44,7 @@ message CreateStreamRequest { // Experimental. Might change in the future. // The Stream that a create made or got. message CreateStreamResponse { - // The ref, with the owner of a bound Stream. + // The ref of the Stream. temporal.api.stream.v1.StreamRef ref = 1; // False when USE_EXISTING got a Stream that existed. bool created = 2; @@ -74,9 +62,6 @@ message AppendRecordsRequest { // the next number of the epoch. Such a batch has no dedupe, so a retry writes it again. int64 sequence = 4; repeated temporal.api.stream.v1.AppendRecord records = 5; - // Only read by the server, on a Workflow Producer's batch. The batch withdraws no Workflow - // Task and keeps no task facts, but the writer's withdrawal withdraw_run still applies. - bool defer_withdrawal = 6; // Only read by the server. The server checks the Producer against the Activity's current // attempt, so nobody else writes as that Activity. bytes task_token = 7; @@ -92,13 +77,10 @@ message AppendRecordsResponse { // Experimental. Might change in the future. // Closes a Stream. Readers drain what is left, then end, and later appends fail with CLOSED. A // close writes no Record and carries no value, so code that has a last value publishes it first. -// Only the owner's run chain closes a bound Stream. Only a Workflow or a client closes a -// standalone Stream. message CloseStreamRequest { string namespace = 1; temporal.api.stream.v1.StreamRef ref = 2; - // The writer that closes the Stream. An SDK Store reads only its Producer, and the server - // also reads its withdrawal fields. + // The writer that closes the Stream. A Store reads only its Producer. temporal.api.stream.v1.StreamWriter writer = 3; // Only read by the server. It checks the token as on the append. bytes task_token = 4; @@ -122,8 +104,7 @@ message ReadRecordsRequest { int64 from_offset = 3; // Where to start when from_offset is zero. Unspecified is the beginning. temporal.api.enums.v1.StreamReadStart start = 4; - // The most items in one page, Superseded items included. Zero is 1000. A Superseded item and - // the Record after it stay in one page, so a page of one Record can hold two items. + // The most Records in one page. Zero is 1000. int32 max_records = 5; // How long the read waits for a new Record before it answers with an empty page. Empty answers // at once, and a wait longer than 60 s waits 60 s. @@ -180,7 +161,7 @@ message DescribeStreamRequest { // The state of a Stream. message DescribeStreamResponse { temporal.api.enums.v1.StreamStatus status = 1; - // The newest offset, of a Record or a Superseded item. Zero when the Stream has none. + // The newest offset, of a Record. Zero when the Stream has none. int64 last_offset = 2; google.protobuf.Duration retention = 3; } diff --git a/crates/streams/README.md b/crates/streams/README.md index 7a6d20b5e..7ce501a4e 100644 --- a/crates/streams/README.md +++ b/crates/streams/README.md @@ -15,11 +15,10 @@ re-exports them, so a value needs no conversion between the crates. - `crates/protos/protos/local/temporal/sdk/core/streams/streams.proto`, in the package `coresdk.streams`, has the crate's own calls, such as the writer calls. - `testdata/*.json` are the shared vectors. They cover Stream ids, Producer ids, Store keys and - meta fields, offsets, epoch keys, the withdrawal rule and the batch digest of a dedupe entry. + meta fields, offsets, epoch keys and the batch digest of a dedupe entry. `testdata/gen_vectors.py` writes them from a second implementation, and the crate's tests read - each file. The Python SDK also reads `ids.json` and `withdrawals.json`. Each Store reads - `fields.json` through the shared Store suite. `digests.json` is the golden of the crate's batch - digest. + each file. The Python SDK also reads `ids.json`. Each Store reads `fields.json` through the + shared Store suite. `digests.json` is the golden of the crate's batch digest. ## The runtime diff --git a/crates/streams/src/epoch.rs b/crates/streams/src/epoch.rs index 15b1ca6a2..16c7f1e86 100644 --- a/crates/streams/src/epoch.rs +++ b/crates/streams/src/epoch.rs @@ -1,4 +1,4 @@ -//! Producer epochs, their order, and the withdrawal rule of a `Superseded` item. +//! Producer epochs and their order. //! //! A Store keeps an epoch as a key of fixed-width big-endian fields, in the order of comparison. //! A byte comparison of two keys then orders the epochs field by field. `testdata/epochs.json` has @@ -6,10 +6,7 @@ use crate::{ StreamError, StreamResult, - proto::{ - Producer, StreamRecord, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, producer::Kind, - stream_record::Item, - }, + proto::{Producer, WorkflowTaskEpoch, producer::Kind}, }; use prost_types::Timestamp; use std::cmp::Ordering; @@ -66,30 +63,6 @@ pub fn producer_run_id(producer: &Producer) -> &str { } } -/// The run that a writer withdraws, empty for none. -pub fn writer_withdraw_run(writer: &StreamWriter) -> &str { - writer - .withdrawal - .as_ref() - .map_or("", |w| w.withdraw_run.as_str()) -} - -/// The started event ids of the run's failed Workflow Tasks that a writer reports. -pub fn writer_failed_tasks(writer: &StreamWriter) -> &[i64] { - writer - .withdrawal - .as_ref() - .map_or(&[], |w| w.failed_tasks.as_slice()) -} - -/// The run's last completed Workflow Task that a writer reports, `None` for none. -pub fn writer_previous_completed_task(writer: &StreamWriter) -> Option<&WorkflowTaskEpoch> { - writer - .withdrawal - .as_ref() - .and_then(|w| w.previous_completed_task.as_ref()) -} - /// The length of a [task_key]. pub const TASK_KEY_LEN: usize = 24; /// The length of an [epoch_key]. @@ -161,79 +134,6 @@ pub fn compare_epochs(a: Option<&Kind>, b: Option<&Kind>) -> Ordering { epoch_key(a).cmp(&epoch_key(b)) } -fn compare_tasks(a: &WorkflowTaskEpoch, b: &WorkflowTaskEpoch) -> Ordering { - task_key(a).cmp(&task_key(b)) -} - -/// Whether a `Superseded` item withdraws an earlier Record. The case of the epoch of the item's -/// writer picks the rule. -/// -/// For an Activity or a Local Activity, it withdraws that Producer's Records with a lower epoch. -/// For a Workflow task t with the previous completed task c, it withdraws the Records of the run -/// and of its Local Activities whose task is in `failed_tasks` or strictly between c and t, and -/// those at c or at t with a lower epoch. The item of a retry run withdraws every Record of the -/// previous run. A client's newer epoch fences the earlier ones but withdraws nothing, since a -/// restarted client's earlier Records are real messages, not a failed attempt's. -pub fn withdraws(superseded: &StreamSuperseded, record: &StreamRecord) -> bool { - let Some(writer) = superseded.writer.as_ref() else { - return false; - }; - let Some(Item::Data(data)) = &record.item else { - return false; - }; - let Some(producer) = data.producer.as_ref() else { - return false; - }; - let withdraw_run = writer_withdraw_run(writer); - if !withdraw_run.is_empty() && producer_run_id(producer) == withdraw_run { - return true; - } - let Some(withdrawing) = writer.producer.as_ref() else { - return false; - }; - let (new, old) = (withdrawing.kind.as_ref(), producer.kind.as_ref()); - let (Some(new_kind), Some(old_kind)) = (new, old) else { - return false; - }; - match ProducerKind::from(new_kind) { - ProducerKind::Client => return false, - ProducerKind::Workflow => {} - same => { - return ProducerKind::from(old_kind) == same - && producer.id == withdrawing.id - && compare_epochs(old, new) == Ordering::Less; - } - } - if !of_run(producer, withdrawing) { - return false; - } - let (task, old_task) = (task_of(new), task_of(old)); - let started = old_task.started_event_id; - if writer_failed_tasks(writer).contains(&started) { - return true; - } - let completed = writer_previous_completed_task(writer); - let after = completed.map_or(0, |c| c.started_event_id); - if after < started && started < task.started_event_id { - return true; - } - let lower_at = |at: &WorkflowTaskEpoch| { - started == at.started_event_id && compare_tasks(&old_task, at) == Ordering::Less - }; - lower_at(&task) || completed.is_some_and(lower_at) -} - -/// Whether a Producer is the Workflow run that withdraws, or one of its Local Activities. -fn of_run(producer: &Producer, withdrawing: &Producer) -> bool { - match kind_of(producer) { - Some(ProducerKind::Workflow) => producer.id == withdrawing.id, - Some(ProducerKind::LocalActivity) => { - producer_run_id(producer) == producer_run_id(withdrawing) - } - _ => false, - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/streams/src/error.rs b/crates/streams/src/error.rs index 7e2e086f2..029614bad 100644 --- a/crates/streams/src/error.rs +++ b/crates/streams/src/error.rs @@ -18,8 +18,7 @@ pub enum Reason { Expired, /// The Stream is closed. Closed, - /// The Producer's epoch is lower than the newest one, its Workflow Task was withdrawn, or a - /// Workflow retry withdrew its run. + /// The Producer's epoch is lower than the newest one. Fenced, /// The batch has a lower `sequence` than the last one of its epoch. SequenceTooLow, diff --git a/crates/streams/src/lib.rs b/crates/streams/src/lib.rs index 6fd9a271e..f2933701c 100644 --- a/crates/streams/src/lib.rs +++ b/crates/streams/src/lib.rs @@ -31,7 +31,10 @@ pub mod proto { pub use temporalio_common::protos::{ coresdk::streams::*, temporal::api::{ - enums::v1::{StreamErrorReason, StreamIdConflictPolicy, StreamReadStart, StreamStatus}, + enums::v1::{ + StreamCloseStatus, StreamErrorReason, StreamIdConflictPolicy, StreamReadStart, + StreamStatus, + }, stream::v1::*, streamservice::v1::*, }, diff --git a/crates/streams/src/memory.rs b/crates/streams/src/memory.rs index de076e762..2d2676530 100644 --- a/crates/streams/src/memory.rs +++ b/crates/streams/src/memory.rs @@ -5,18 +5,17 @@ use crate::{ Reason, StreamError, StreamResult, digest::DIGEST_LEN, - epoch::{ - EPOCH_KEY_LEN, ProducerKind, TASK_KEY_LEN, epoch_key, producer_run_id, task_key, task_of, - }, - ids::{StreamKeys, encode_part}, + epoch::{EPOCH_KEY_LEN, ProducerKind, epoch_key}, + ids::StreamKeys, offset::{entry_id_of_offset, offset_of_entry_id}, - proto::{Producer, StreamRef, WorkflowTaskEpoch}, + proto::{Producer, StreamCloseStatus, StreamRef}, store::{ Appended, Batch, Capabilities, Closed, ConflictPolicy, Create, CreateOutcome, Created, - Entry, FORMAT_VERSION, MAX_SEQUENCE, Owner, Store, StreamState, TOMBSTONE, owner_chain, + Entry, FORMAT_VERSION, MAX_SEQUENCE, Store, StreamState, TOMBSTONE, }, }; use parking_lot::Mutex; +use prost::Message; use std::{ collections::{BTreeMap, HashMap}, time::{Duration, SystemTime, UNIX_EPOCH}, @@ -31,7 +30,6 @@ const PRUNE_LIMIT: usize = 64; #[derive(Clone, Default)] struct Fields { record: Option>, - superseded: Option>, } /// One Stream's meta hash and log, each with an expiry time, as Redis keys have. @@ -92,22 +90,18 @@ fn parse_id(text: &str) -> Option<(u64, u64)> { } /// The length of a [ProducerField] value. -const PRODUCER_FIELD_LEN: usize = 1 + 2 * EPOCH_KEY_LEN + 8 + 2 * 12 + DIGEST_LEN; -/// The flag of a [ProducerField] that has a held epoch. The other flag bits are zero. -const HELD: u8 = 1; +const PRODUCER_FIELD_LEN: usize = EPOCH_KEY_LEN + 8 + 2 * 12 + DIGEST_LEN; /// An entry id as a [ProducerField] keeps it, the ms and the seq. pub(crate) type EntryId = (u64, u32); -/// The `p||` field, one per Producer. Its value is 129 bytes: a flag byte, the -/// fence, the held epoch key or zeros, and the last batch's sequence (8), first and last entry ids -/// (12 each, the ms in 8 and the seq in 4, zeros for none) and digest. Numbers are big-endian. +/// The `p||` field, one per Producer. Its value is 88 bytes: the fence, and the +/// last batch's sequence (8), first and last entry ids (12 each, the ms in 8 and the seq in 4, zeros +/// for none) and digest. Numbers are big-endian. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct ProducerField { /// The newest epoch key of the Producer, which is also the epoch of its last batch. pub(crate) fence: [u8; EPOCH_KEY_LEN], - /// The newest epoch key that still has Records. - pub(crate) held: Option<[u8; EPOCH_KEY_LEN]>, /// The last batch's sequence. pub(crate) sequence: i64, /// The last batch's first and last entry ids, `None` when it wrote no entry. @@ -119,9 +113,7 @@ pub(crate) struct ProducerField { impl ProducerField { pub(crate) fn encode(&self) -> Vec { let mut value = Vec::with_capacity(PRODUCER_FIELD_LEN); - value.push(if self.held.is_some() { HELD } else { 0 }); value.extend_from_slice(&self.fence); - value.extend_from_slice(&self.held.unwrap_or([0; EPOCH_KEY_LEN])); value.extend_from_slice(&self.sequence.to_be_bytes()); let (first, last) = self.entries.unwrap_or_default(); for (ms, seq) in [first, last] { @@ -133,7 +125,7 @@ impl ProducerField { } } -/// Refuses a value of another length or with another flag bit, which a later format may write. +/// Refuses a value of another length, which a later format may write. impl TryFrom<&[u8]> for ProducerField { type Error = StreamError; @@ -144,14 +136,10 @@ impl TryFrom<&[u8]> for ProducerField { "the Producer field has an unknown format", ) }; - let Some((&flags, rest)) = value.split_first() else { - return Err(unknown()); - }; - if value.len() != PRODUCER_FIELD_LEN || flags & !HELD != 0 { + if value.len() != PRODUCER_FIELD_LEN { return Err(unknown()); } - let (fence, rest) = rest.split_at(EPOCH_KEY_LEN); - let (held, rest) = rest.split_at(EPOCH_KEY_LEN); + let (fence, rest) = value.split_at(EPOCH_KEY_LEN); let (sequence, rest) = rest.split_at(8); let (first, rest) = rest.split_at(12); let (last, digest) = rest.split_at(12); @@ -169,11 +157,6 @@ impl TryFrom<&[u8]> for ProducerField { }; Ok(Self { fence: fence.try_into().map_err(|_| unknown())?, - held: if flags & HELD != 0 { - Some(held.try_into().map_err(|_| unknown())?) - } else { - None - }, sequence: i64::from_be_bytes(sequence.try_into().map_err(|_| unknown())?), entries, digest: digest.try_into().map_err(|_| unknown())?, @@ -191,88 +174,12 @@ fn entry_id(text: &str) -> StreamResult { )) } -type TaskKey = [u8; TASK_KEY_LEN]; - -/// The started event id of a task key, its first 8 bytes. -fn started(key: &TaskKey) -> i64 { - let mut id = [0; 8]; - id.copy_from_slice(&key[..8]); - i64::from_be_bytes(id) -} - -/// Whether the facts of task `t` show that task `key` never completed. An orphan item withdraws the -/// Records of such a task. -fn never_completed(key: &TaskKey, t: &TaskKey, c: Option<&TaskKey>, failed: &[i64]) -> bool { - let (s, ts) = (started(key), started(t)); - let cs = c.map_or(0, started); - failed.contains(&s) - || (cs < s && s < ts) - || (s == ts && key < t) - || c.is_some_and(|c| s == cs && key < c) -} - -/// The orphan rule over a run's open Local Activity tasks, as `orphan_tasks` in `lib.lua`. -fn orphan_tasks( - open: &[TaskKey], - t: &TaskKey, - c: Option<&TaskKey>, - failed: &[i64], -) -> (Vec, Vec) { - let (mut withdrawn, mut kept) = (vec![], vec![]); - for key in open { - if never_completed(key, t, c, failed) { - withdrawn.push(*key); - } else if c.is_none_or(|c| key > c) { - kept.push(*key); - } - } - (withdrawn, kept) -} - -/// The task key of the Workflow Task `[started_event_id]`, with no attempt and no time. -fn started_key(started_event_id: i64) -> TaskKey { - task_key(&WorkflowTaskEpoch { - started_event_id, - ..Default::default() - }) -} - -/// The task keys of a field that keeps them back to back, such as `oa|` or `tf|`. -fn task_keys(value: &[u8]) -> StreamResult> { - let (keys, rest) = value.as_chunks::(); - if !rest.is_empty() { - return Err(refuse( - Reason::StorageCorrupt, - "a field of task keys ends in a partial key", - )); - } - Ok(keys.to_vec()) -} - -/// Whether the facts that `tf|` keeps show that task `key` never completed. The zero key in -/// the completed place works as no completed task, since no task key is below it. -fn fenced_by_facts(facts: &[TaskKey], key: &TaskKey) -> StreamResult { - let [t, c, failed @ ..] = facts else { - return Err(refuse( - Reason::StorageCorrupt, - "the run's task facts have fewer than two task keys", - )); - }; - let failed: Vec = failed.iter().map(started).collect(); - Ok(never_completed(key, t, Some(c), &failed)) -} - -/// The `ow||` field, set when a `Superseded` item withdrew the run's task. -fn withdrawn_field(run: &str, task: &TaskKey) -> Vec { - [format!("ow|{run}|").as_bytes(), task].concat() -} - impl Keys { fn get(&self, field: impl AsRef<[u8]>) -> Option<&[u8]> { self.meta.get(field.as_ref()).map(Vec::as_slice) } - /// A field that holds text, such as `own` or `newest`. `None` when it's absent or not text. + /// A field that holds text, such as `newest`. `None` when it's absent or not text. fn text(&self, field: impl AsRef<[u8]>) -> Option<&str> { self.get(field) .and_then(|value| std::str::from_utf8(value).ok()) @@ -315,36 +222,14 @@ impl Keys { Ok(()) } - /// The `check_owner` step of a close, as in `lib.lua`. - fn check_owner(&self, kind: ProducerKind, chain: &str) -> StreamResult<()> { - if self.text("own") == Some("wf") { - if kind != ProducerKind::Workflow { - return Err(refuse( - Reason::NotOwner, - "only the close Activity of the owner run chain closes a Workflow-owned Stream", - )); - } - if self.text("oc").is_some_and(|owner| chain != owner) { - return Err(refuse( - Reason::NotOwner, - "the ref has another run chain than the owner", - )); - } - } else if !matches!(kind, ProducerKind::Workflow | ProducerKind::Client) { - return Err(refuse( - Reason::NotOwner, - "only a Workflow or a client closes a Stream with no owner", - )); - } - Ok(()) - } - fn refresh(&mut self, now: i64, safety: i64, tombstone: i64) { let retention = self.number("ret"); - let mut ttl = retention; - if self.text("own") == Some("wf") && self.get("closed").is_none() { - ttl = safety; - } + // An open Stream keeps the safety TTL after its last write. A closed one keeps the Retention. + let ttl = if self.get("closed").is_none() { + safety + } else { + retention + }; let exp = now.saturating_add(ttl); self.set("exp", exp.to_string()); self.meta_expires = exp.saturating_add(tombstone); @@ -387,19 +272,6 @@ impl Keys { now.max(self.newest_id().0 as i64) } - /// Refuses a create that names another owner than the Stream has, as `check_identity` in - /// `lib.lua`. - fn check_identity(&self, owner: &str, chain: &str) -> StreamResult<()> { - let stored = self.text("oc").unwrap_or_default(); - if self.text("own") != Some(owner) || stored != chain { - return Err(refuse( - Reason::NotOwner, - "the Stream has another owner than the create names", - )); - } - Ok(()) - } - fn trim(&mut self, now: i64) { if self.log.is_none() { // The expired log took every Record, so a read from an older offset must fail. @@ -439,14 +311,6 @@ impl Keys { } } - fn withdraw_run(&mut self, run: &str) -> bool { - if run.is_empty() || self.get(format!("wr|{run}")).is_some() { - return false; - } - self.set(format!("wr|{run}"), "1"); - self.get(format!("run|{run}")).is_some() - } - /// Refuses `count` entries whose ids would not fit an offset. The ids go on from the newest id /// the Stream gave, as `check_ids_fit` in `lib.lua`. fn check_ids_fit(&self, now: i64, count: u64) -> StreamResult<()> { @@ -583,14 +447,6 @@ impl MemoryStore { } } -fn encode_run(run_id: &str) -> String { - if run_id.is_empty() { - String::new() - } else { - encode_part(run_id) - } -} - fn offsets(first: &str, last: &str) -> StreamResult> { if first.is_empty() || last.is_empty() { return Ok(None); @@ -617,72 +473,34 @@ impl Store for MemoryStore { let retention = create.retention.filter(|r| !r.is_zero()); let retention = millis(retention.unwrap_or(self.retention)); let request_id = create.request_id; - let owner = match create.owner { - Owner::Workflow => "wf", - Owner::Client => "client", - }; let (safety, tombstone) = (millis(self.safety_ttl), millis(TOMBSTONE)); - let withdrawal = &create.withdrawal; - let withdrawn_run = encode_run(withdrawal.withdraw_run); - let chain = if owner == "wf" { - owner_chain(stream_ref) - } else { - String::new() - }; let created = self.with(namespace, stream_id, |keys, now| { if keys.get("ret").is_none() { keys.set("v", FORMAT_VERSION.to_string()); keys.set("ret", retention.to_string()); - keys.set("own", owner); - if !chain.is_empty() { - keys.set("oc", chain.clone()); - } if !request_id.is_empty() { keys.set("rid", request_id); } - keys.withdraw_run(&withdrawn_run); keys.refresh(now, safety, tombstone); - return Ok((CreateOutcome::Created, retention, false)); + return Ok((CreateOutcome::Created, retention)); } keys.check_live(now)?; - keys.check_identity(owner, &chain)?; if keys.get("closed").is_some() { return Err(refuse(Reason::Closed, "the Stream is closed")); } // A retry of the create that made the Stream gets its answer again. if !request_id.is_empty() && keys.get("rid") == Some(request_id.as_bytes()) { - return Ok((CreateOutcome::Created, keys.number("ret"), false)); + return Ok((CreateOutcome::Created, keys.number("ret"))); } if create.conflict == ConflictPolicy::Fail { return Err(refuse(Reason::AlreadyExists, "the Stream exists")); } - if !withdrawn_run.is_empty() { - keys.check_ids_fit(now, 1)?; - } - let mut flagged = false; - if keys.withdraw_run(&withdrawn_run) { - keys.trim(now); - let id = keys.add( - now, - Fields { - record: None, - superseded: Some(withdrawal.superseded.to_vec()), - }, - ); - keys.set("newest", id); - keys.refresh(now, safety, tombstone); - flagged = true; - } - Ok((CreateOutcome::Exists, keys.number("ret"), flagged)) + Ok((CreateOutcome::Exists, keys.number("ret"))) }); - let (outcome, stored, flagged) = created?; - if flagged { - self.appended.notify_waiters(); - } + let (outcome, stored) = created?; Ok(Created { outcome, retention: Duration::from_millis(stored.max(0) as u64), - flagged, }) } @@ -696,22 +514,9 @@ impl Store for MemoryStore { let producer = batch.producer; let kind = ProducerKind::try_from(producer)?; let epoch = epoch_key(producer.kind.as_ref()); - let run = encode_run(producer_run_id(producer)); - let withdrawal = &batch.withdrawal; - let withdrawn_run = encode_run(withdrawal.withdraw_run); let (safety, tombstone) = (millis(self.safety_ttl), millis(TOMBSTONE)); let field = format!("p|{}|{}", kind as i32, producer.id); let member = format!("{}|{}", kind as i32, producer.id); - // The Workflow Task of a Local Activity of a run, which the run's task facts can withdraw. - let task = (kind == ProducerKind::LocalActivity && !run.is_empty()) - .then(|| task_key(&task_of(producer.kind.as_ref()))); - let facts = withdrawal.tasks.filter(|_| task.is_some()); - let completed = facts.and_then(|facts| facts.completed).map(task_key); - let open_field = format!("oa|{run}"); - // The task facts of the run's highest task, as task keys back to back. These are its own - // key, the completed task's key or the zero key before the first, and the key of each - // failed task's started event id. - let facts_field = format!("tf|{run}"); let appended = self.with(namespace, stream_id, |keys, now| { // The format version comes first, since a newer format can change the Producer field. keys.check_live(now)?; @@ -746,7 +551,7 @@ impl Store for MemoryStore { (format!("{ms}-{seq}"), format!("{last_ms}-{last_seq}")) }) .unwrap_or_default(); - return Ok((true, first, last, false)); + return Ok((true, first, last)); } if sequence < previous.sequence { return Err(refuse( @@ -761,115 +566,30 @@ impl Store for MemoryStore { if keys.get("closed").is_some() { return Err(refuse(Reason::Closed, "the Stream is closed")); } - if !run.is_empty() && keys.get(format!("wr|{run}")).is_some() { - return Err(refuse( - Reason::Fenced, - "a Workflow retry withdrew the run of the Producer", - )); - } if stored.is_some_and(|stored| epoch < stored.fence) { return Err(refuse( Reason::Fenced, "the epoch is lower than the newest one of the Producer", )); } - if let Some(task) = task { - if keys.get(withdrawn_field(&run, &task)).is_some() { - return Err(refuse( - Reason::Fenced, - "a Superseded item withdrew the Workflow Task of the call", - )); - } - // A task can fail before its call writes, so no orphan item withdrew it. - let newest = keys.get(&facts_field).map(task_keys).transpose()?; - if let Some(newest) = newest - && fenced_by_facts(&newest, &task)? - { - return Err(refuse( - Reason::Fenced, - "the run's task facts show that the Workflow Task of the call never \ - completed", - )); - } - } let records = batch.records.len() as u64; - keys.check_ids_fit(now, records + 2)?; - - // A client's newer epoch only fences, as in `append.lua`. - let newer = kind != ProducerKind::Client - && stored - .and_then(|stored| stored.held) - .is_some_and(|held| held < epoch); - let mut flagged = newer; - if keys.withdraw_run(&withdrawn_run) { - flagged = true; - } - let open = match task { - Some(_) => keys - .get(&open_field) - .map(task_keys) - .transpose()? - .unwrap_or_default(), - None => vec![], - }; - let (withdrawn, mut open) = match (task, facts) { - (Some(task), Some(facts)) => { - orphan_tasks(&open, &task, completed.as_ref(), facts.failed_tasks) - } - _ => (vec![], open), - }; + keys.check_ids_fit(now, records)?; keys.trim(now); keys.prune(now); let at = keys.next_ms(now); let (mut first, mut last) = (String::new(), String::new()); - // The orphan item has an entry of its own, as in `append.lua`. - let mut orphan = String::new(); - if let (false, Some(facts)) = (withdrawn.is_empty(), facts) { - orphan = keys.add( - now, - Fields { - record: None, - superseded: Some(facts.superseded.to_vec()), - }, - ); - last = orphan.clone(); - } - if records == 0 && flagged { - last = keys.add( - now, - Fields { - record: None, - superseded: Some(withdrawal.superseded.to_vec()), - }, - ); - first = last.clone(); - } - for (i, record) in batch.records.iter().enumerate() { - let superseded = (i == 0 && flagged).then(|| withdrawal.superseded.to_vec()); + for record in batch.records { last = keys.add( now, Fields { record: Some(record.clone()), - superseded, }, ); if first.is_empty() { first = last.clone(); } } - - if first.is_empty() { - first = orphan; - } - // Only a newer epoch's item withdraws the held epoch's Records. - let held = if records > 0 { - Some(epoch) - } else if newer { - None - } else { - stored.and_then(|stored| stored.held) - }; let entries = if last.is_empty() { None } else { @@ -877,64 +597,25 @@ impl Store for MemoryStore { }; let value = ProducerField { fence: epoch, - held, sequence, entries, digest: batch.digest, }; keys.set(field.as_str(), value.encode()); - if !run.is_empty() && records > 0 { - keys.set(format!("run|{run}"), "1"); - } - if let Some(task) = task { - // A write with no task facts, as from an older Core, goes in `oa` too, so that a - // later write's facts can withdraw it. - let added = records > 0 && !open.contains(&task); - if added { - open.push(task); - } - if facts.is_some() || added { - if open.is_empty() { - keys.remove(&open_field); - } else { - keys.set(open_field.as_str(), open.concat()); - } - } - // One field per task, so the fence reads one field, as in `append.lua`. - for key in &withdrawn { - keys.set(withdrawn_field(&run, key), "1"); - } - } - if let (Some(task), Some(facts)) = (task, facts) { - // A long Local Activity of an earlier task writes with older facts. - let newest = keys.get(&facts_field).map(task_keys).transpose()?; - let highest = newest.as_ref().and_then(|newest| newest.first()); - if highest.is_none_or(|highest| *highest < task) { - let completed = completed.unwrap_or_else(|| started_key(0)); - let failed = facts.failed_tasks.iter().map(|&failed| started_key(failed)); - let value: Vec = [task, completed] - .into_iter() - .chain(failed) - .flatten() - .collect(); - keys.set(facts_field.as_str(), value); - } - } if !last.is_empty() { keys.set("newest", last.clone()); } keys.producers.insert(member.clone(), at); keys.refresh(now, safety, tombstone); - Ok((false, first, last, flagged || !withdrawn.is_empty())) + Ok((false, first, last)) }); - let (resend, first, last, flagged) = appended?; + let (resend, first, last) = appended?; if !resend && !last.is_empty() { self.appended.notify_waiters(); } Ok(Appended { offsets: offsets(&first, &last)?, resend, - flagged, }) } @@ -947,22 +628,13 @@ impl Store for MemoryStore { let stream_id = stream_ref.id.as_str(); let kind = ProducerKind::try_from(producer)?; let epoch = epoch_key(producer.kind.as_ref()); - let run = encode_run(producer_run_id(producer)); let field = format!("p|{}|{}", kind as i32, producer.id); let tombstone = millis(TOMBSTONE); - let chain = owner_chain(stream_ref); let closed = self.with(namespace, stream_id, |keys, now| { keys.check_live(now)?; - keys.check_owner(kind, &chain)?; if keys.get("closed").is_some() { return Ok(true); } - if !run.is_empty() && keys.get(format!("wr|{run}")).is_some() { - return Err(refuse( - Reason::Fenced, - "a Workflow retry withdrew the run of the Producer", - )); - } let stored = keys.get(&field).map(ProducerField::try_from).transpose()?; if stored.is_some_and(|stored| epoch < stored.fence) { return Err(refuse( @@ -971,6 +643,10 @@ impl Store for MemoryStore { )); } keys.set("closed", "1"); + // A read builds the final StreamClose item from the closer and the status. A Brook + // Store only ever closes with CLOSED. + keys.set("closer", producer.encode_to_vec()); + keys.set("cstat", (StreamCloseStatus::Closed as i32).to_string()); keys.refresh(now, 0, tombstone); Ok(false) }); @@ -1003,6 +679,8 @@ impl Store for MemoryStore { exists: keys.get("v").is_some(), version: number("v") as u32, closed: keys.get("closed").is_some(), + closer: keys.get("closer").map(<[u8]>::to_vec), + close_status: number("cstat") as i32, newest: offset("newest")?, trimmed: offset("trimmed")?, retention: Duration::from_millis(number("ret").max(0) as u64), @@ -1015,7 +693,6 @@ impl Store for MemoryStore { entries.push(Entry { offset: offset_of_entry_id(&id_text(*id))?, record: fields.record.clone(), - superseded: fields.superseded.clone(), }); } } @@ -1042,7 +719,6 @@ impl Store for MemoryStore { Ok(Entry { offset: offset_of_entry_id(&id_text(*id))?, record: fields.record.clone(), - superseded: fields.superseded.clone(), }) }) .collect() @@ -1117,7 +793,6 @@ mod tests { }); let field = ProducerField { fence: key("fence").unwrap(), - held: key("held"), sequence: case["sequence"].as_i64().unwrap(), entries, digest: hex(text("digest")).try_into().unwrap(), diff --git a/crates/streams/src/metrics.rs b/crates/streams/src/metrics.rs index 42824b437..5fa5fb14d 100644 --- a/crates/streams/src/metrics.rs +++ b/crates/streams/src/metrics.rs @@ -8,7 +8,6 @@ use temporalio_common::telemetry::metrics::{ pub(crate) struct Metrics { attributes: MetricAttributes, fenced: Counter, - superseded: Counter, blocking_reads_waiting: UpDownCounter, } @@ -30,12 +29,7 @@ impl Metrics { attributes: meter.get_default_attributes().clone(), fenced: meter.counter(parameters( "stream_fenced", - "Batches that the Store refused for a lower epoch or a withdrawn run", - "", - )), - superseded: meter.counter(parameters( - "stream_superseded", - "Superseded flags that the Store set", + "Batches that the Store refused for a lower epoch", "", )), blocking_reads_waiting: meter.up_down_counter(parameters( @@ -50,10 +44,6 @@ impl Metrics { self.fenced.add(1, &self.attributes); } - pub(crate) fn superseded(&self) { - self.superseded.add(1, &self.attributes); - } - pub(crate) fn read_waits(&self, waiting: bool) { self.blocking_reads_waiting .add(if waiting { 1 } else { -1 }, &self.attributes); diff --git a/crates/streams/src/record.rs b/crates/streams/src/record.rs index 2277adf9b..e10ed22ae 100644 --- a/crates/streams/src/record.rs +++ b/crates/streams/src/record.rs @@ -1,11 +1,8 @@ -//! The stored Records and the `Superseded` facts of a batch. +//! The stored Records of a batch. use crate::{ Reason, StreamError, StreamResult, - proto::{ - AppendRecord, Producer, RunWithdrawal, StreamData, StreamRecord, StreamSuperseded, - StreamWriter, stream_record::Item, - }, + proto::{AppendRecord, Producer, StreamData, StreamRecord, stream_record::Item}, store::{MAX_BATCH_BYTES, MAX_BATCH_RECORDS, MAX_RECORD_BYTES}, }; use prost::Message; @@ -63,21 +60,6 @@ pub(crate) fn data_records( Ok(records) } -/// The facts a `Superseded` item of this batch gives the Reader. -pub(crate) fn superseded(producer: &Producer, withdraw_run: &str) -> Vec { - let withdrawal = (!withdraw_run.is_empty()).then(|| RunWithdrawal { - withdraw_run: withdraw_run.to_string(), - ..Default::default() - }); - StreamSuperseded { - writer: Some(StreamWriter { - producer: Some(producer.clone()), - withdrawal, - }), - } - .encode_to_vec() -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index b074cbe53..e25c056ad 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -4,23 +4,19 @@ use crate::{ Reason, StreamError, StreamResult, digest::batch_digest, - epoch::{ - ProducerKind, producer_run_id, task_of, withdraws, writer_failed_tasks, - writer_previous_completed_task, writer_withdraw_run, - }, + epoch::{ProducerKind, task_of}, ids::{check_producer, check_stream_id}, metrics::Metrics, proto::{ AppendRecordsRequest, AppendRecordsResponse, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, - Producer, ReadRecordsRequest, ReadRecordsResponse, RunWithdrawal, StreamIdConflictPolicy, - StreamReadStart, StreamRecord, StreamRef, StreamStatus, StreamSuperseded, StreamWriter, - WorkflowProducer, WorkflowTaskEpoch, producer::Kind, stream_record::Item, + Producer, ReadRecordsRequest, ReadRecordsResponse, StreamClose, StreamIdConflictPolicy, + StreamReadStart, StreamRecord, StreamRef, StreamStatus, stream_record::Item, }, - record::{data_records, superseded}, + record::data_records, store::{ Batch, Capabilities, ConflictPolicy, Create, CreateOutcome, FORMAT_VERSION, MAX_SEQUENCE, - MAX_TTL, MIN_RETENTION, Owner, Store, StreamState, TaskFacts, Withdrawal, owner_chain, + MAX_TTL, MIN_RETENTION, Store, StreamState, }, }; use prost::Message; @@ -107,45 +103,6 @@ fn stream_id(stream_ref: Option<&StreamRef>) -> StreamResult<&str> { Ok(id) } -/// The `Superseded` facts of a create that withdraws a retry run's previous run. It has no epoch, -/// so a Reader withdraws only that run's Records. -fn run_superseded(owner: Option<&Producer>, withdraw_run: &str) -> Vec { - let producer = Producer { - kind: None, - ..owner.cloned().unwrap_or_default() - }; - superseded(&producer, withdraw_run) -} - -/// The `Superseded` facts that withdraw a run's orphaned Local Activity Records. Their writer is -/// the run's Workflow Producer in the batch's task, so that a Reader applies the Workflow rule. -fn orphan_superseded( - producer: &Producer, - completed: Option<&WorkflowTaskEpoch>, - failed_tasks: &[i64], -) -> Vec { - let run_id = producer_run_id(producer); - let task = task_of(producer.kind.as_ref()); - let workflow = Producer { - id: crate::ids::workflow_producer_id(run_id), - kind: Some(Kind::Workflow(WorkflowProducer { - run_id: run_id.to_string(), - epoch: Some(task), - })), - }; - StreamSuperseded { - writer: Some(StreamWriter { - producer: Some(workflow), - withdrawal: Some(RunWithdrawal { - previous_completed_task: completed.copied(), - failed_tasks: failed_tasks.to_vec(), - withdraw_run: String::new(), - }), - }), - } - .encode_to_vec() -} - /// Checks a Producer that writes outside Workflow code. fn outside_producer(producer: Option<&Producer>) -> StreamResult<&Producer> { let producer = producer.ok_or_else(|| StreamError::invalid("The request has no Producer"))?; @@ -166,52 +123,7 @@ fn outside_producer(producer: Option<&Producer>) -> StreamResult<&Producer> { Ok(producer) } -/// Refuses task facts that a Store would drop or misread. Only a Local Activity has its run's task -/// facts, and a failed task is a started event id, which is never zero or negative. -fn check_task_facts(kind: ProducerKind, writer: &StreamWriter) -> StreamResult<()> { - let completed = writer_previous_completed_task(writer); - if completed.is_none() && writer_failed_tasks(writer).is_empty() { - return Ok(()); - } - if kind != ProducerKind::LocalActivity { - return Err(StreamError::invalid( - "Only a Local Activity sends its run's task facts", - )); - } - if completed.is_none() { - return Err(StreamError::invalid( - "Failed tasks need the run's last completed task", - )); - } - if completed.is_some_and(|c| c.started_event_id < 0) - || writer_failed_tasks(writer).iter().any(|&id| id <= 0) - { - return Err(StreamError::invalid( - "A task's started event id must be positive", - )); - } - Ok(()) -} - -/// Checks the owner of a create, which must be a Workflow run with a ref of its run chain. A -/// Store stores that chain and checks every later create and close against it. -fn create_owner<'a>(owner: &'a Producer, stream_ref: &StreamRef) -> StreamResult<&'a Producer> { - check_producer(owner)?; - if ProducerKind::try_from(owner)? != ProducerKind::Workflow { - return Err(StreamError::invalid( - "The owner of a create is a Workflow Producer", - )); - } - if owner_chain(stream_ref).is_empty() { - return Err(StreamError::invalid( - "A create with an owner needs a ref whose owner is the Workflow's run chain", - )); - } - Ok(owner) -} - -/// Checks the Producer of a close. A Workflow's close Activity closes as the run's Workflow -/// Producer, so a Workflow Producer may close. +/// Checks the Producer of a close. fn close_producer(producer: Option<&Producer>) -> StreamResult<&Producer> { let producer = producer.ok_or_else(|| StreamError::invalid("The request has no Producer"))?; check_producer(producer)?; @@ -227,54 +139,18 @@ impl StreamService { let stream = request.stream_ref.clone().unwrap_or_default(); let retention = retention(request.retention)?; let conflict = conflict_policy(request.id_conflict_policy)?; - // A Workflow's call from outside Workflow code, such as from a Local Activity, sets the - // owner run. Then the Stream has the safety TTL of a Workflow-owned Stream. - let owner = match &request.owner { - Some(owner) => { - create_owner(owner, &stream)?; - self.store - .capabilities() - .require(Capabilities::SAFETY_TTL)?; - Owner::Workflow - } - None => Owner::Client, - }; - if !request.withdraw_run.is_empty() { - // The `Superseded` item names its Producer, which here can only be the owner. Any other - // caller could withdraw and fence an unrelated run, as on the append. - let Some(owner) = &request.owner else { - return Err(StreamError::invalid( - "A create with withdraw_run needs the owner run", - )); - }; - if request.withdraw_run == producer_run_id(owner) { - return Err(StreamError::invalid("A run doesn't withdraw itself")); - } - self.store.capabilities().require(Capabilities::RUN_FENCE)?; - } - let bytes = run_superseded(request.owner.as_ref(), &request.withdraw_run); let create = Create { retention, - owner, conflict, request_id: &request.request_id, - withdrawal: Withdrawal { - withdraw_run: &request.withdraw_run, - superseded: &bytes, - tasks: None, - }, }; let created = self .store .create(&request.namespace, &stream, &create) .await?; - if created.flagged { - self.metrics.superseded(); - } Ok(CreateStreamResponse { r#ref: Some(StreamRef { id: stream.id, - owner: stream.owner.filter(|_| owner == Owner::Workflow), backend: self.backend.clone(), }), created: created.outcome == CreateOutcome::Created, @@ -289,16 +165,15 @@ impl StreamService { let stream = request.r#ref.clone().unwrap_or_default(); let writer = request.writer.unwrap_or_default(); let producer = outside_producer(writer.producer.as_ref())?; - let kind = ProducerKind::try_from(producer)?; if !(0..=MAX_SEQUENCE).contains(&request.sequence) { return Err(StreamError::invalid( "The sequence must be from 1 to 2^53 - 1, or zero for the next one", )); } - // Without the fence and the flag, a copy or a stale batch would reach Readers. + // Without the fence, a stale batch would reach Readers. self.store .capabilities() - .require(Capabilities::PRODUCER_FENCE.union(Capabilities::WITHDRAWAL))?; + .require(Capabilities::PRODUCER_FENCE)?; if request.sequence == 0 { // A call with no writer state sends no sequence. Only the Store can number the batch in // the same step as its dedupe check. @@ -306,41 +181,13 @@ impl StreamService { .capabilities() .require(Capabilities::NEXT_SEQUENCE)?; } - let withdraw_run = writer_withdraw_run(&writer); - if !withdraw_run.is_empty() { - // Only a retry run's Local Activity withdraws its previous run, as only the owner's - // create does. Any other Producer could withdraw and fence an unrelated run. - if kind != ProducerKind::LocalActivity || withdraw_run == producer_run_id(producer) { - return Err(StreamError::invalid( - "Only a Local Activity of a later run withdraws a run", - )); - } - self.store.capabilities().require(Capabilities::RUN_FENCE)?; - } - check_task_facts(kind, &writer)?; let digest = batch_digest(&request.records); let records = data_records(producer, request.records)?; - let bytes = superseded(producer, withdraw_run); - // A Local Activity sends its run's task facts, so that the batch withdraws the run's - // orphaned Local Activity Records. These are the Records of the tasks that never completed. - let completed = writer_previous_completed_task(&writer).filter(|c| c.started_event_id > 0); - let orphans = (kind == ProducerKind::LocalActivity - && writer_previous_completed_task(&writer).is_some()) - .then(|| orphan_superseded(producer, completed, writer_failed_tasks(&writer))); let batch = Batch { producer, sequence: request.sequence, records: &records, digest, - withdrawal: Withdrawal { - withdraw_run, - superseded: &bytes, - tasks: orphans.as_ref().map(|orphans| TaskFacts { - completed, - failed_tasks: writer_failed_tasks(&writer), - superseded: orphans, - }), - }, }; let appended = self.store.append(&request.namespace, &stream, &batch).await; let appended = self.count(appended)?; @@ -351,12 +198,12 @@ impl StreamService { }) } - /// Counts a fence or a flag of an append on the metrics. - pub(crate) fn count(&self, result: StreamResult) -> StreamResult { - match &result { - Ok(value) if value.flagged() => self.metrics.superseded(), - Err(error) if error.reason == Reason::Fenced => self.metrics.fenced(), - _ => {} + /// Counts a fence of an append on the metrics. + fn count(&self, result: StreamResult) -> StreamResult { + if let Err(error) = &result + && error.reason == Reason::Fenced + { + self.metrics.fenced(); } result } @@ -488,9 +335,8 @@ impl StreamService { } } - /// The newest `last` Records, oldest first, and `done` for a closed Stream. A Record that a - /// newer `Superseded` item withdraws doesn't count. It never waits, and it stops after - /// [MAX_SCAN] entries or [MAX_SCAN_BYTES] of them. + /// The newest `last` Records, oldest first, and `done` for a closed Stream. It never waits, and + /// it stops after [MAX_SCAN] entries or [MAX_SCAN_BYTES] of them. async fn read_last( &self, request: ReadRecordsRequest, @@ -514,7 +360,6 @@ impl StreamService { check_expiry(&state, 0)?; let retained = state.retained_from(); let mut picked = vec![]; - let mut withdrawals: Vec = vec![]; let (mut bytes, mut scanned, mut scanned_bytes) = (0, 0, 0); let mut before = state.newest; 'scan: while before >= retained @@ -537,30 +382,20 @@ impl StreamService { } before = entry.offset - 1; scanned += 1; - scanned_bytes += entry.record.as_ref().map_or(0, Vec::len) - + entry.superseded.as_ref().map_or(0, Vec::len); - // An entry's `Superseded` item withdraws earlier Records, not the entry's own. - if let Some(record) = entry.record { - let record = decode(&record, entry.offset)?; + scanned_bytes += entry.record.as_ref().map_or(0, Vec::len); + if let Some(encoded) = entry.record { + let record = decode(&encoded, entry.offset)?; let size = record.encoded_len(); - if !withdrawals.iter().any(|s| withdraws(s, &record)) { - // A Record over the cap is still returned alone, so that the read always - // returns a Record. - if !picked.is_empty() && bytes + size > max_bytes { - break 'scan; - } - bytes += size; - picked.push(record); - if picked.len() == count { - break 'scan; - } + // A Record over the cap is still returned alone, so that the read always + // returns a Record. + if !picked.is_empty() && bytes + size > max_bytes { + break 'scan; + } + bytes += size; + picked.push(record); + if picked.len() == count { + break 'scan; } - } - if let Some(superseded) = entry.superseded { - withdrawals.push( - StreamSuperseded::decode(superseded.as_slice()) - .map_err(|error| StreamError::storage_corrupt(error.to_string()))?, - ); } } } @@ -620,6 +455,9 @@ impl StreamService { let mut pulled = (0, 0); // Whether the Stream was closed, and its newest offset, at the newest Store read. let mut end = (state.closed, state.newest); + // The closer and the status at the newest Store read, for the final close item. + let mut closer = state.closer.clone(); + let mut close_status = state.close_status; while records.len() < max && !full { let room = (max_bytes != usize::MAX).then(|| max_bytes.saturating_sub(bytes)); let chunk = chunk_len(max - records.len(), room, pulled); @@ -627,6 +465,8 @@ impl StreamService { // A trim after the state read leaves a gap before these entries. check_expiry(&chunk_state, if from > 0 { start } else { 0 })?; end = (chunk_state.closed, chunk_state.newest); + close_status = chunk_state.close_status; + closer = chunk_state.closer; if entries.is_empty() { // Only an expired log has no entry at an offset up to `newest`. if from > 0 && start == from && from <= chunk_state.newest { @@ -643,30 +483,24 @@ impl StreamService { start = next; continue; } - let mut items = vec![]; - if let Some(bytes) = entry.superseded { - let superseded = StreamSuperseded::decode(bytes.as_slice()) - .map_err(|error| StreamError::storage_corrupt(error.to_string()))?; - items.push(StreamRecord { - offset: entry.offset, - item: Some(Item::Superseded(superseded)), - }); - } - if let Some(bytes) = entry.record { - items.push(decode(&bytes, entry.offset)?); - } + let Some(encoded) = entry.record else { + next = entry.offset + 1; + start = next; + continue; + }; + let record = decode(&encoded, entry.offset)?; // An entry over the caps is still returned alone, so that the read always makes // progress. The next read starts at the entry that didn't fit. - let size: usize = items.iter().map(Message::encoded_len).sum(); + let size = record.encoded_len(); pulled = (pulled.0 + size, pulled.1 + 1); - let over = bytes + size > max_bytes || records.len() + items.len() > max; + let over = bytes + size > max_bytes || records.len() + 1 > max; if !records.is_empty() && over { full = true; next = entry.offset; break; } bytes += size; - records.extend(items); + records.push(record); next = entry.offset + 1; start = next; if records.len() >= max { @@ -679,6 +513,23 @@ impl StreamService { if records.is_empty() && !done { return Ok(None); } + // The read that reaches the end of a closed Stream ends with its close item, at the offset + // after the newest Record. A read that starts past the newest Record already saw it, so it + // doesn't repeat it, and `next_offset` stays at the close offset. + if done && from <= end.1 { + let producer = closer + .as_deref() + .map(Producer::decode) + .transpose() + .map_err(|error| StreamError::storage_corrupt(error.to_string()))?; + records.push(StreamRecord { + offset: end.1 + 1, + item: Some(Item::Close(StreamClose { + producer, + status: close_status, + })), + }); + } let page = ReadRecordsResponse { records, next_offset: next, @@ -734,14 +585,3 @@ fn max_bytes(value: i64) -> StreamResult { fn expired() -> StreamError { StreamError::new(Reason::Expired, "The Records at the offset expired") } - -/// A Store answer that may have withdrawn Records. -pub(crate) trait Flagged { - fn flagged(&self) -> bool; -} - -impl Flagged for crate::store::Appended { - fn flagged(&self) -> bool { - self.flagged - } -} diff --git a/crates/streams/src/store.rs b/crates/streams/src/store.rs index 7fb71ee0e..7a0d44961 100644 --- a/crates/streams/src/store.rs +++ b/crates/streams/src/store.rs @@ -4,18 +4,15 @@ use crate::{ Reason, StreamError, StreamResult, - ids::encode_part, - proto::{Producer, StreamRef, WorkflowTaskEpoch}, + proto::{Producer, StreamRef}, }; use std::time::Duration; -use temporalio_common::protos::temporal::api::enums::v1::ExecutionType; use tokio::time::Instant; /// The Retention of a Stream whose create sets none, when a Store's options set no other. pub const DEFAULT_RETENTION: Duration = Duration::from_secs(7 * 24 * 60 * 60); -/// How long an open Workflow-owned Stream stays after its last write, when a Store's options set -/// no other. +/// How long an open Stream stays after its last write, when a Store's options set no other. pub const DEFAULT_SAFETY_TTL: Duration = Duration::from_secs(90 * 24 * 60 * 60); /// The format version this crate writes. A Stream with a higher one refuses writes. @@ -39,33 +36,6 @@ pub const MAX_TTL: Duration = Duration::from_secs(315_576_000_000); /// next write. pub const MIN_RETENTION: Duration = Duration::from_millis(1); -/// The owner run chain of a ref as `:`, each part encoded, or empty -/// for another owner. A Store keeps it on a Workflow-owned Stream and checks creates and closes on -/// it. -pub fn owner_chain(stream_ref: &StreamRef) -> String { - stream_ref - .owner - .as_ref() - .filter(|owner| owner.r#type == ExecutionType::Workflow as i32) - .map_or_else(String::new, |owner| { - format!( - "{}:{}", - encode_part(&owner.business_id), - encode_part(&owner.run_id) - ) - }) -} - -/// Who created a Stream, which decides its TTL while it is open. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] -pub enum Owner { - /// Until it closes, the Stream expires `safety_ttl` after its last write. - Workflow, - /// The Stream has no owner. It expires one Retention after its last write. - #[default] - Client, -} - /// What a create does when its Stream exists and is open, and the create isn't a retry of the /// create that made the Stream. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] @@ -82,15 +52,11 @@ pub enum ConflictPolicy { pub struct Create<'a> { /// The Retention, `None` for the Store default. pub retention: Option, - /// Who creates the Stream. - pub owner: Owner, /// What the create does when the Stream exists. pub conflict: ConflictPolicy, /// Unique for each create call, and the same on its retries. Empty for none. The Store keeps /// the request id of the create that made the Stream, so a retry after a lost answer succeeds. pub request_id: &'a str, - /// What a retry run's create withdraws. - pub withdrawal: Withdrawal<'a>, } /// What a create found. @@ -110,38 +76,6 @@ pub struct Created { pub outcome: CreateOutcome, /// The Retention of the Stream, the stored one if it existed. pub retention: Duration, - /// Whether the create withdrew a retry run's previous run. - pub flagged: bool, -} - -/// The withdrawal facts of a batch, and the `Superseded` item that they make. -#[derive(Debug, Clone, Default)] -pub struct Withdrawal<'a> { - /// A retry run's previous run. - pub withdraw_run: &'a str, - /// The encoded `Superseded` facts, which the log keeps when the batch withdraws something. - pub superseded: &'a [u8], - /// The run's task facts of a Local Activity batch, which withdraw the run's orphaned Local - /// Activity Records and fence their later writes. - pub tasks: Option>, -} - -/// The run's Workflow Task facts that a Local Activity batch gives. A task of the run after -/// `completed` and before the batch's own task, or one in `failed_tasks`, never completed, so -/// its Local Activity Records are orphans. -/// -/// A Store keeps the task of each Local Activity batch with Records, also of one with no facts, so -/// that a later batch's facts withdraw it. It also keeps the facts of the run's highest task and -/// fences a write whose task they show never completed, since a task can fail before its call -/// writes. -#[derive(Debug, Clone, Copy, Default)] -pub struct TaskFacts<'a> { - /// The run's last completed Workflow Task, `None` before the first one. - pub completed: Option<&'a WorkflowTaskEpoch>, - /// The started event ids of the run's failed Workflow Tasks. - pub failed_tasks: &'a [i64], - /// The encoded Workflow-kind `Superseded` facts that withdraw the orphans. - pub superseded: &'a [u8], } /// One batch of one Producer. @@ -156,20 +90,15 @@ pub struct Batch<'a> { pub records: &'a [Vec], /// The [crate::digest::batch_digest] of the batch, which the dedupe entry keeps. pub digest: [u8; crate::digest::DIGEST_LEN], - /// What the batch withdraws. - pub withdrawal: Withdrawal<'a>, } /// The result of an append. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Appended { - /// The offsets of the first and the last entry, `None` for an empty batch with no - /// `Superseded` item. + /// The offsets of the first and the last entry, `None` for an empty batch. pub offsets: Option<(i64, i64)>, /// The Store already had the batch, so the offsets are those of the first result. pub resend: bool, - /// The batch withdrew earlier Records. - pub flagged: bool, } /// The result of a close. @@ -188,6 +117,12 @@ pub struct StreamState { pub version: u32, /// The Stream is closed. Every later append fails, so `newest` is final. pub closed: bool, + /// The encoded [Producer] that closed the Stream, `None` when it is open. A read builds the + /// final `StreamClose` item from it. + pub closer: Option>, + /// Why the Stream closed, as a `StreamCloseStatus`, zero when it is open. A Brook Store only + /// sets CLOSED. + pub close_status: i32, /// The offset of the newest entry written, zero if none. pub newest: i64, /// The offset of the newest entry that Retention dropped, zero if none. @@ -223,10 +158,8 @@ impl StreamState { pub struct Entry { /// Its offset. pub offset: i64, - /// The encoded Record, absent on an entry that only withdraws. + /// The encoded Record. pub record: Option>, - /// The encoded `Superseded` facts of a flagged batch. - pub superseded: Option>, } /// The rules that a Store applies atomically beyond the log. The service checks a capability before @@ -238,28 +171,18 @@ pub struct Capabilities(u8); impl Capabilities { /// Each append checks the Producer's epoch and its last `sequence` before it writes. pub const PRODUCER_FENCE: Self = Self(1); - /// Each append flags the first batch of a Producer's newer epoch, which withdraws the - /// Records of its earlier ones. - pub const WITHDRAWAL: Self = Self(1 << 1); - /// A retry run's first write withdraws the previous run and fences its Producers. - pub const RUN_FENCE: Self = Self(1 << 2); /// A close marks the Stream closed and wakes the waiting reads. Every later append is /// refused. - pub const CLOSE: Self = Self(1 << 3); - /// An open Workflow-owned Stream keeps the safety TTL, not the Retention. - pub const SAFETY_TTL: Self = Self(1 << 4); + pub const CLOSE: Self = Self(1 << 1); /// An append with sequence zero takes the next one of the Producer's epoch, in the same atomic /// step as the dedupe check. So two such batches never get the same number. - pub const NEXT_SEQUENCE: Self = Self(1 << 5); + pub const NEXT_SEQUENCE: Self = Self(1 << 2); /// Every capability. - pub const ALL: Self = Self((1 << 6) - 1); + pub const ALL: Self = Self((1 << 3) - 1); - const NAMES: [(Self, &'static str); 6] = [ + const NAMES: [(Self, &'static str); 3] = [ (Self::PRODUCER_FENCE, "producer_fence"), - (Self::WITHDRAWAL, "withdrawal"), - (Self::RUN_FENCE, "run_fence"), (Self::CLOSE, "close"), - (Self::SAFETY_TTL, "safety_ttl"), (Self::NEXT_SEQUENCE, "next_sequence"), ]; @@ -316,14 +239,11 @@ pub trait Store: Send + Sync + 'static { /// another one must say what it forwards. fn capabilities(&self) -> Capabilities; - /// Creates a Stream. The calls that write take the Stream's whole ref, since the server - /// finds a Workflow's Stream by its owner. + /// Creates a Stream. /// - /// The create of an existing Stream fails with [Reason::Expired], [Reason::NotOwner] or - /// [Reason::Closed], in this order. Then a retry of the create that made the Stream, with the - /// same request id, gets [CreateOutcome::Created] and changes nothing. Any other create - /// applies its [ConflictPolicy]. With [Capabilities::RUN_FENCE], a retry run's create - /// withdraws its previous run. + /// The create of an existing Stream fails with [Reason::Expired] or [Reason::Closed], in this + /// order. Then a retry of the create that made the Stream, with the same request id, gets + /// [CreateOutcome::Created] and changes nothing. Any other create applies its [ConflictPolicy]. async fn create( &self, namespace: &str, @@ -394,9 +314,13 @@ mod tests { let some = Capabilities::PRODUCER_FENCE.union(Capabilities::CLOSE); some.require(Capabilities::CLOSE).unwrap(); let error = some - .require(Capabilities::CLOSE.union(Capabilities::SAFETY_TTL)) + .require(Capabilities::CLOSE.union(Capabilities::NEXT_SEQUENCE)) .unwrap_err(); assert_eq!(error.reason, Reason::Unsupported); - assert!(error.message.ends_with("safety_ttl"), "{}", error.message); + assert!( + error.message.ends_with("next_sequence"), + "{}", + error.message + ); } } diff --git a/crates/streams/src/testing/fields.rs b/crates/streams/src/testing/fields.rs index 720d70543..83839bf7e 100644 --- a/crates/streams/src/testing/fields.rs +++ b/crates/streams/src/testing/fields.rs @@ -1,54 +1,22 @@ //! The fields of the `:meta` hash. The field names hold encoded ids, so a `|` in a field name is a //! separator. Only an `ow|` name ends in a binary task key, after its last separator. -use crate::{epoch::TASK_KEY_LEN, ids::encode_part}; - /// The format version of the Stream. pub const VERSION: &str = "v"; -/// The field of a Producer. Its value is 129 bytes, with big-endian numbers: +/// The field of a Producer. Its value is 88 bytes, with big-endian numbers: /// -/// - 1, the flags. Bit 0 is set when the held epoch is present, and the other bits are zero. /// - 40, the fence, the Producer's newest epoch key. It is also the epoch of the last batch. -/// - 40, the held epoch key, the newest one that still has Records, or zeros. /// - 8, the last batch's sequence. /// - 12 and 12, the last batch's first and last entry ids, each the ms in 8 bytes and the seq in /// 4. They are zero when the batch wrote no entry. /// - 16, the last batch's digest, which a resend must match. /// -/// A Store refuses a value of another length or with another flag bit. +/// A Store refuses a value of another length. pub fn producer(kind: i32, producer_id: &str) -> String { format!("p|{kind}|{producer_id}") } -/// Set when a Producer of the run wrote a Record to the Stream. -pub fn run_wrote(run_id: &str) -> String { - format!("run|{}", encode_part(run_id)) -} - -/// Set when a Workflow retry withdrew the run. -pub fn withdrawn_run(run_id: &str) -> String { - format!("wr|{}", encode_part(run_id)) -} - -/// The task keys of the run's Local Activity writes that no task facts showed completed yet, back -/// to back. -pub fn open_tasks(run_id: &str) -> String { - format!("oa|{}", encode_part(run_id)) -} - -/// Set when a `Superseded` item withdrew the run's Workflow Task with the key `task_key`. -pub fn withdrawn_task(run_id: &str, task_key: &[u8; TASK_KEY_LEN]) -> Vec { - [format!("ow|{}|", encode_part(run_id)).as_bytes(), task_key].concat() -} - -/// The task facts of the run's highest task, as task keys back to back. These are its own key, the -/// completed task's key or the zero key before the first, and the key of each failed task's -/// started event id. -pub fn task_facts(run_id: &str) -> String { - format!("tf|{}", encode_part(run_id)) -} - #[cfg(test)] mod tests { use super::*; @@ -59,16 +27,6 @@ mod tests { serde_json::from_str(include_str!("../../testdata/fields.json")).unwrap(); let text = |field: &str| case[field].as_str().unwrap(); let kind = case["kind"].as_i64().unwrap() as i32; - let (producer_id, run) = (text("producer_id"), text("run_id")); - assert_eq!(producer(kind, producer_id), text("producer")); - assert_eq!(run_wrote(run), text("run_wrote")); - assert_eq!(withdrawn_run(run), text("withdrawn_run")); - assert_eq!(open_tasks(run), text("open_tasks")); - let task_key = crate::testing::support::hex(text("task_key")); - assert_eq!( - withdrawn_task(run, &task_key.as_slice().try_into().unwrap()), - [text("withdrawn_task_prefix").as_bytes(), &task_key].concat() - ); - assert_eq!(task_facts(run), text("task_facts")); + assert_eq!(producer(kind, text("producer_id")), text("producer")); } } diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 7cc54d9fe..4660578da 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -6,20 +6,17 @@ use super::{case, support::Needs}; use crate::{ StreamError, StreamRuntime, - epoch::{withdraws, writer_withdraw_run}, - ids::{activity_producer_id, client_producer_id, named_client_producer_id}, + ids::{client_producer_id, named_client_producer_id}, proto::{ ActivityEpoch, ActivityProducer, AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, ClientProducer, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, - LocalActivityProducer, Producer, ReadRecordsRequest, ReadRecordsResponse, RunWithdrawal, - StreamErrorDetails, StreamIdConflictPolicy, StreamReadStart, StreamRecord, StreamRef, - StreamStatus, StreamSuperseded, StreamWriter, WorkflowProducer, WorkflowTaskEpoch, - producer::Kind, stream_record::Item, + LocalActivityProducer, Producer, ReadRecordsRequest, ReadRecordsResponse, StreamCloseStatus, + StreamErrorDetails, StreamIdConflictPolicy, StreamReadStart, StreamRef, StreamStatus, + StreamWriter, WorkflowProducer, WorkflowTaskEpoch, producer::Kind, stream_record::Item, }, }; use prost::Message; -use prost_types::Timestamp; use std::{ sync::Arc, time::{Duration, Instant}, @@ -31,8 +28,6 @@ struct Case { ext: Arc, namespace: String, stream: String, - /// What the backend gives, for a step that needs more than the case asked for. - gives: Needs, } static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); @@ -57,7 +52,6 @@ async fn cases_for(needs: Needs) -> Vec { ext: runtime.ext, namespace: runtime.namespace.unwrap_or(format!("ns-{unique}")), stream: format!("s-{unique}"), - gives: runtime.gives, } }) .collect() @@ -78,11 +72,10 @@ fn reason(error: &StreamError) -> String { error.reason.token().to_string() } -/// The writer of a write by `producer`, with no withdrawal facts. +/// The writer of a write by `producer`. fn writer_for(producer: Producer) -> Option { Some(StreamWriter { producer: Some(producer), - ..Default::default() }) } @@ -103,24 +96,6 @@ fn named(name: &str, attempt: i32) -> Producer { } } -/// An Activity Producer of a made-up run, at `attempt`. A higher attempt withdraws the lower ones' -/// Records. The server requires the Activity's task token, so only Redis and memory accept it. -fn activity(attempt: i32, scheduled: i64) -> Producer { - Producer { - id: activity_producer_id("r1", "a", "s"), - kind: Some(Kind::Activity(ActivityProducer { - run_id: "r1".to_string(), - epoch: Some(ActivityEpoch { - attempt_scheduled_time: Some(Timestamp { - seconds: scheduled, - nanos: 0, - }), - attempt, - }), - })), - } -} - /// The Producer of run `run` in its first Workflow Task, as its close Activity sends it. fn workflow(run: &str) -> Producer { Producer { @@ -256,19 +231,12 @@ fn texts(response: &ReadRecordsResponse) -> Vec { .iter() .map(|record| match &record.item { Some(Item::Data(data)) => String::from_utf8(data.body.clone().unwrap().data).unwrap(), - Some(Item::Superseded(_)) => "".to_string(), + Some(Item::Close(_)) => "".to_string(), None => "?".to_string(), }) .collect() } -fn superseded_of(record: &StreamRecord) -> &StreamSuperseded { - match &record.item { - Some(Item::Superseded(superseded)) => superseded, - other => panic!("not a Superseded item: {other:?}"), - } -} - /// A create that uses a Stream that exists returns it with `created` false, whatever its /// Retention. A Retention outside a proto Duration's range, or under 1 ms, fails. async fn a_create_returns_the_stream_that_exists_whatever_its_retention() { @@ -380,36 +348,12 @@ async fn a_close_writes_no_record_and_a_second_close_succeeds() { } } -/// A Stream with no owner takes a close from a Workflow or a client. The close of an Activity -/// fails as `not_owner`. -async fn a_workflow_or_a_client_closes_a_stream_with_no_owner() { - for case in cases_for(Needs::UNCHECKED).await { - case.create(None).await.unwrap(); - let run = format!("{}-run", case.stream); - let activity = Producer { - id: activity_producer_id(&run, "a", "s"), - kind: Some(Kind::Activity(ActivityProducer { - run_id: run.clone(), - epoch: Some(ActivityEpoch { - attempt: 1, - ..Default::default() - }), - })), - }; - let error = case.close(activity).await.unwrap_err(); - assert_eq!( - (error.code(), reason(&error).as_str()), - (Code::FailedPrecondition, "stream_not_owner") - ); - case.close(workflow(&run)).await.unwrap(); - } -} - -/// A create of a Stream whose Retention passed fails as expired while its Tombstone is there, not -/// as `already_exists`. +/// A create of a closed Stream whose Retention passed fails as expired while its Tombstone is +/// there, before the closed check and before `already_exists`. async fn a_create_of_an_expired_stream_fails() { for case in cases_for(Needs::SHORT_TIMES).await { case.create(millis(300)).await.unwrap(); + case.close(client(1)).await.unwrap(); tokio::time::sleep(Duration::from_millis(400)).await; let error = case.create(None).await.unwrap_err(); assert_eq!( @@ -419,82 +363,6 @@ async fn a_create_of_an_expired_stream_fails() { } } -/// A create with an owner needs a Workflow Producer and a ref of its run chain, and the answer's -/// ref has that chain. A create with another owner fails as `not_owner`, not as -/// `already_exists`, and no run withdraws itself. -async fn a_create_checks_its_owner_and_its_ref() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - let workflow = workflow(&run); - let task = WorkflowTaskEpoch { - started_event_id: 3, - ..Default::default() - }; - let local_activity = Producer { - id: crate::ids::local_activity_producer_id(&run, "call-1"), - kind: Some(Kind::LocalActivity(LocalActivityProducer { - run_id: run.clone(), - epoch: Some(LocalActivityEpoch { - task: Some(task), - attempt: 1, - }), - })), - }; - let chain = |workflow_id: &str| StreamRef { - id: case.stream.clone(), - owner: super::support::workflow_execution(workflow_id, "first-run"), - ..Default::default() - }; - let create = |owner: Option<&Producer>, stream_ref: StreamRef, withdraw_run: &str| { - CreateStreamRequest { - namespace: case.namespace.clone(), - owner: owner.cloned(), - stream_ref: Some(stream_ref), - withdraw_run: withdraw_run.to_string(), - ..Default::default() - } - }; - for request in [ - create(Some(&local_activity), chain("agent"), ""), - create(Some(&workflow), case.stream_ref().unwrap(), ""), - create( - Some(&workflow), - StreamRef { - id: String::new(), - ..chain("agent") - }, - "", - ), - create(Some(&workflow), chain("agent"), &run), - ] { - let error = call::<_, CreateStreamResponse>(&case.ext, "CreateStream", request) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_invalid"); - } - let created: CreateStreamResponse = call( - &case.ext, - "CreateStream", - create(Some(&workflow), chain("agent"), ""), - ) - .await - .unwrap(); - assert_eq!(created.r#ref.unwrap().owner, chain("agent").owner); - for request in [ - create(Some(&workflow), chain("another-agent"), ""), - create(None, case.stream_ref().unwrap(), ""), - ] { - let error = call::<_, CreateStreamResponse>(&case.ext, "CreateStream", request) - .await - .unwrap_err(); - assert_eq!( - (error.code(), reason(&error).as_str()), - (Code::FailedPrecondition, "stream_not_owner") - ); - } - } -} - /// A Producer's epoch has a case, its id must match that kind, a Workflow appends only through a /// Local Activity, and a Local Activity's epoch names the Workflow Task that dispatched it. async fn a_producer_must_match_its_kind_and_not_be_a_workflow() { @@ -615,42 +483,6 @@ async fn a_read_is_inclusive_from_its_offset_and_get_offset_reads_only_new_recor } } -/// The first batch of a higher Activity attempt puts its `Superseded` item just before its -/// Records, and a page keeps the two together under `max_records`. -async fn a_higher_attempt_puts_its_superseded_item_before_its_batch() { - for case in cases_for(Needs::UNCHECKED).await { - case.create(None).await.unwrap(); - case.append(activity(1, 10), 1, vec![record("old")]) - .await - .unwrap(); - case.append(activity(2, 20), 1, vec![record("new1"), record("new2")]) - .await - .unwrap(); - let all = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&all), ["old", "", "new1", "new2"]); - // The item is just before the batch's first Record. Redis stores both in one entry with one - // offset, and the server gives the item its own offset just below. - let superseded = &all.records[1]; - assert!(all.records[0].offset < superseded.offset); - assert!(superseded.offset <= all.records[2].offset); - let writer = superseded_of(superseded).writer.as_ref().unwrap(); - assert_eq!(writer.producer.as_ref().unwrap().id, activity(2, 20).id); - let mut from_offset = 0; - for expected in [&["old"][..], &["", "new1"], &["new2"]] { - let page = case - .read(ReadRecordsRequest { - from_offset, - max_records: 2, - ..Default::default() - }) - .await - .unwrap(); - assert_eq!(texts(&page), expected); - from_offset = page.next_offset; - } - } -} - /// A read waits for the Stream's create only as long as `create_wait`. async fn a_read_waits_for_the_create_only_as_long_as_create_wait() { for case in cases_for(Needs::CALLS).await { @@ -753,7 +585,7 @@ async fn a_close_ends_every_read_with_done() { case.append(client(1), 1, vec![record("1")]).await.unwrap(); case.close(client(1)).await.unwrap(); let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["1"]); + assert_eq!(texts(&read), ["1", ""]); assert!(read.done); let again = case .read(ReadRecordsRequest { @@ -779,6 +611,25 @@ async fn a_close_ends_every_read_with_done() { } } +/// A read of a closed Stream ends with a StreamClose item, at the offset after the newest Record. +/// Its status is CLOSED, and its Producer is the one that closed the Stream. +async fn a_closed_read_ends_with_a_close_item() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let written = case.append(client(1), 1, vec![record("1")]).await.unwrap(); + case.close(client(1)).await.unwrap(); + let read = case.read(ReadRecordsRequest::default()).await.unwrap(); + assert!(read.done); + let last = read.records.last().expect("a close item"); + assert_eq!(last.offset, written.last_offset + 1); + let Some(Item::Close(close)) = &last.item else { + panic!("not a close item: {:?}", last.item); + }; + assert_eq!(close.status, StreamCloseStatus::Closed as i32); + assert_eq!(close.producer, Some(client(1))); + } +} + /// An unknown method fails with UNIMPLEMENTED. async fn an_unknown_method_is_unimplemented() { for case in cases_for(Needs::CALLS).await { @@ -787,30 +638,6 @@ async fn an_unknown_method_is_unimplemented() { } } -/// A read of an open Stream that expired fails at once instead of waiting. -async fn a_read_of_an_expired_open_stream_fails_instead_of_waiting() { - for case in cases_for(Needs::SHORT_TIMES).await { - case.create(millis(300)).await.unwrap(); - case.append(client(1), 1, vec![record("1")]).await.unwrap(); - tokio::time::sleep(Duration::from_millis(400)).await; - for from_offset in [0, 1] { - let started = Instant::now(); - let error = case - .read(ReadRecordsRequest { - from_offset, - wait: millis(5000), - ..Default::default() - }) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_expired"); - assert!(started.elapsed() < Duration::from_secs(2)); - } - let error = case.append(client(1), -1, vec![]).await.unwrap_err(); - assert_eq!(reason(&error), "stream_invalid"); - } -} - /// A read of a closed Stream whose Retention passed fails as expired, from any offset, instead /// of waiting out its wait. A last read fails the same way. async fn a_read_of_an_expired_closed_stream_fails_instead_of_waiting() { @@ -854,29 +681,17 @@ async fn a_read_of_an_expired_closed_stream_fails_instead_of_waiting() { } } -/// Creates the case's Stream, owned by `run`, with a 300 ms Retention. While it is open, its meta -/// stays and only its log expires. -async fn create_owned_short(case: &Case, run: &str) { - let _: CreateStreamResponse = call( - &case.ext, - "CreateStream", - CreateStreamRequest { - namespace: case.namespace.clone(), - retention: millis(300), - owner: Some(workflow(run)), - stream_ref: owned_ref(case), - ..Default::default() - }, - ) - .await - .unwrap(); +/// Creates the case's Stream with a 300 ms Retention. While it is open, its meta stays and only +/// its log expires. +async fn create_short(case: &Case) { + case.create(millis(300)).await.unwrap(); } /// A read from an offset that an idle open Stream's expired log took fails as expired at once, /// before the next write and after it. A read from the start skips the gap. async fn a_read_behind_an_expired_log_fails_before_and_after_the_next_write() { - for case in cases_for(Needs::UNCHECKED.and(Needs::SHORT_TIMES)).await { - create_owned_short(&case, &format!("{}-run", case.stream)).await; + for case in cases_for(Needs::SHORT_TIMES).await { + create_short(&case).await; let first = case .append(client(1), 1, vec![record("a"), record("b")]) .await @@ -902,21 +717,14 @@ async fn a_read_behind_an_expired_log_fails_before_and_after_the_next_write() { /// A close after an idle open Stream's log expired keeps the gap, so a read from an offset that the /// log took fails as expired. async fn a_close_after_the_log_expired_keeps_the_gap() { - for case in cases_for(Needs::UNCHECKED.and(Needs::SHORT_TIMES)).await { - let run = format!("{}-run", case.stream); - create_owned_short(&case, &run).await; + for case in cases_for(Needs::SHORT_TIMES).await { + create_short(&case).await; let first = case .append(client(1), 1, vec![record("a"), record("b")]) .await .unwrap(); tokio::time::sleep(Duration::from_millis(400)).await; - let close = CloseStreamRequest { - namespace: case.namespace.clone(), - r#ref: owned_ref(&case), - writer: writer_for(workflow(&run)), - ..Default::default() - }; - let _: CloseStreamResponse = call(&case.ext, "CloseStream", close).await.unwrap(); + case.close(client(1)).await.unwrap(); let behind = ReadRecordsRequest { from_offset: first.first_offset + 1, ..Default::default() @@ -926,47 +734,6 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { } } -/// A ref with an owner and one without reach the same Stream. -async fn a_ref_with_an_owner_and_one_without_reach_the_same_stream() { - for case in cases_for(Needs::ID_KEYS).await { - case.create(None).await.unwrap(); - let owned = Some(StreamRef { - id: case.stream.clone(), - owner: super::support::workflow_execution("agent-1", "run-1"), - ..Default::default() - }); - // An owner that doesn't match the id changes nothing either. - let other = Some(StreamRef { - id: case.stream.clone(), - owner: super::support::workflow_execution("someone-else", ""), - ..Default::default() - }); - for (sequence, stream) in [(1, owned.clone()), (2, case.stream_ref()), (3, other)] { - let request = AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: stream, - writer: writer_for(client(1)), - sequence, - records: vec![record(&sequence.to_string())], - ..Default::default() - }; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", request).await.unwrap(); - } - let read = |stream_ref: Option| ReadRecordsRequest { - namespace: case.namespace.clone(), - r#ref: stream_ref, - ..Default::default() - }; - let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.stream_ref())) - .await - .unwrap(); - let with_owner: ReadRecordsResponse = - call(&case.ext, "ReadRecords", read(owned)).await.unwrap(); - assert_eq!(texts(&plain), ["1", "2", "3"]); - assert_eq!(plain, with_owner); - } -} - /// A create answer names the configured backend, and the Store never reads it. async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { for ext in super::support::named_runtimes("redis-a") { @@ -975,7 +742,6 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { ext, namespace: format!("ns-{unique}"), stream: format!("s-{unique}"), - gives: Needs::ALL, }; let created = case.create(None).await.unwrap(); let stream = created.r#ref.unwrap(); @@ -986,7 +752,6 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { Some(StreamRef { id: case.stream.clone(), backend: backend.to_string(), - ..Default::default() }) }; for (sequence, stream) in [(1, named("redis-a")), (2, named("")), (3, named("other"))] { @@ -1015,769 +780,6 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { } } -/// The Local Activity Producer of a Workflow call in run `run`. -fn local_activity(run: &str) -> Producer { - user_local_activity(run, 3, 100) -} - -/// The case's ref with the owner of a made-up run chain, which a create with an owner needs. -fn owned_ref(case: &Case) -> Option { - Some(StreamRef { - id: case.stream.clone(), - owner: super::support::workflow_execution(&format!("{}-wf", case.stream), "first-run"), - ..Default::default() - }) -} - -/// A retry run's Local Activity calls withdraw the previous run. -async fn a_retry_run_s_local_activity_calls_withdraw_the_previous_run() { - for case in cases_for(Needs::UNCHECKED).await { - let (previous, retry) = (format!("{}-r1", case.stream), format!("{}-r2", case.stream)); - let create = |run: &str, withdraw_run: &str| CreateStreamRequest { - namespace: case.namespace.clone(), - withdraw_run: withdraw_run.to_string(), - owner: Some(workflow(run)), - stream_ref: owned_ref(&case), - ..use_existing() - }; - let append = |run: &str, withdraw_run: &str, text: &str| AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: Some(StreamWriter { - producer: Some(local_activity(run)), - withdrawal: Some(RunWithdrawal { - withdraw_run: withdraw_run.to_string(), - ..Default::default() - }), - }), - sequence: 1, - records: vec![record(text)], - ..Default::default() - }; - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&previous, "")) - .await - .unwrap(); - let _: AppendRecordsResponse = - call(&case.ext, "AppendRecords", append(&previous, "", "old")) - .await - .unwrap(); - // The retry run's first create withdraws the previous run. - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&retry, &previous)) - .await - .unwrap(); - let first: AppendRecordsResponse = - call(&case.ext, "AppendRecords", append(&retry, &previous, "new")) - .await - .unwrap(); - // A retried Local Activity sends the same batch and gets its first answer. - let again: AppendRecordsResponse = - call(&case.ext, "AppendRecords", append(&retry, &previous, "new")) - .await - .unwrap(); - assert_eq!( - (again.first_offset, again.last_offset), - (first.first_offset, first.last_offset) - ); - assert!(first.last_offset > 0); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["old", "", "new"]); - let superseded = superseded_of(&read.records[1]); - let writer = superseded.writer.as_ref().unwrap(); - assert_eq!(writer_withdraw_run(writer), previous); - // The item's writer has its Producer, which the WIT requires. A create's item has no kind, - // so a Reader can't read it as an epoch, and the run stays in the id. - let producer = writer.producer.as_ref().unwrap(); - assert_eq!(producer.id, crate::ids::workflow_producer_id(&retry)); - assert!(producer.kind.is_none()); - let error = call::<_, CreateStreamResponse>( - &case.ext, - "CreateStream", - CreateStreamRequest { - owner: None, - ..create(&retry, &previous) - }, - ) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_invalid"); - assert!(withdraws(superseded, &read.records[0])); - assert!(!withdraws(superseded, &read.records[2])); - } -} - -/// Only a Local Activity of a later run withdraws a run on append. A client, an Activity or the -/// run's own Local Activity that sends a run to withdraw fails with `stream_invalid` and withdraws -/// nothing. -async fn only_a_later_run_s_local_activity_withdraws_a_run_on_append() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 3, 2, "workflow output"), - ) - .await - .unwrap(); - for producer in [client(1), activity(1, 10), local_activity(&run)] { - let error = call::<_, AppendRecordsResponse>( - &case.ext, - "AppendRecords", - AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: Some(StreamWriter { - producer: Some(producer), - withdrawal: Some(RunWithdrawal { - withdraw_run: run.clone(), - ..Default::default() - }), - }), - sequence: 1, - records: vec![record("other")], - ..Default::default() - }, - ) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_invalid"); - } - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["workflow output"]); - let next = local_activity_write(&case, &run, "call-2", 4, 3, "next"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", next).await.unwrap(); - } -} - -/// The user Local Activity `call-1` of `run`, dispatched by the Workflow Task `[started]`. A rerun -/// in a later task keeps the Producer id and has a newer dispatching task. -fn user_local_activity(run: &str, started: i64, seconds: i64) -> Producer { - let task = WorkflowTaskEpoch { - started_event_id: started, - attempt: 1, - started_time: Some(Timestamp { seconds, nanos: 0 }), - }; - Producer { - id: crate::ids::local_activity_producer_id(run, "call-1"), - kind: Some(Kind::LocalActivity(LocalActivityProducer { - run_id: run.to_string(), - epoch: Some(LocalActivityEpoch { - task: Some(task), - attempt: 1, - }), - })), - } -} - -/// A Local Activity rerun in a later task supersedes its first attempt. -async fn a_rerun_local_activity_supersedes_its_first_attempt() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - let _: CreateStreamResponse = call( - &case.ext, - "CreateStream", - CreateStreamRequest { - namespace: case.namespace.clone(), - owner: Some(workflow(&run)), - stream_ref: owned_ref(&case), - ..Default::default() - }, - ) - .await - .unwrap(); - let write = |producer: Producer, text: &str| AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: writer_for(producer), - sequence: 1, - records: vec![record(text)], - ..Default::default() - }; - // Task [3] runs the Local Activity, which writes, and then the task fails. - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - write(user_local_activity(&run, 3, 100), "first"), - ) - .await - .unwrap(); - // The next task [7] runs the same call again, and it writes again. - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - write(user_local_activity(&run, 7, 200), "second"), - ) - .await - .unwrap(); - // A late write of the failed attempt is fenced. - let late = call::<_, AppendRecordsResponse>( - &case.ext, - "AppendRecords", - AppendRecordsRequest { - sequence: 2, - ..write(user_local_activity(&run, 3, 100), "late") - }, - ) - .await - .unwrap_err(); - assert_eq!(reason(&late), "stream_fenced"); - // A Reader sees the first attempt, then the Superseded item that withdraws it, then the - // second attempt. After it applies the item, only the second attempt is visible. - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["first", "", "second"]); - let superseded = superseded_of(&read.records[1]); - assert!(withdraws(superseded, &read.records[0])); - assert!(!withdraws(superseded, &read.records[2])); - } -} - -/// A write of the Local Activity `call` of `run`, dispatched by task `[started]`, with the run's -/// last completed task `[completed]`, or 0 before the first. -fn local_activity_write( - case: &Case, - run: &str, - call: &str, - started: i64, - completed: i64, - text: &str, -) -> AppendRecordsRequest { - let mut producer = user_local_activity(run, started, 100 + started); - producer.id = crate::ids::local_activity_producer_id(run, call); - AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: Some(StreamWriter { - producer: Some(producer), - withdrawal: Some(RunWithdrawal { - previous_completed_task: Some(WorkflowTaskEpoch { - started_event_id: completed, - ..Default::default() - }), - ..Default::default() - }), - }), - sequence: 1, - records: vec![record(text)], - ..Default::default() - } -} - -/// The writer of a write, to change its Producer. -fn writer_mut(request: &mut AppendRecordsRequest) -> &mut StreamWriter { - request.writer.get_or_insert_with(Default::default) -} - -/// The withdrawal facts of a write, to change its task facts. -fn withdrawal_mut(request: &mut AppendRecordsRequest) -> &mut RunWithdrawal { - writer_mut(request) - .withdrawal - .get_or_insert_with(Default::default) -} - -/// The close Activity closes a Workflow's Stream as the run's Workflow Producer, which the service -/// refuses on an append. -async fn the_close_activity_closes_a_workflow_stream_as_its_workflow_producer() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - let chain = StreamRef { - id: case.stream.clone(), - owner: super::support::workflow_execution(&format!("{}-wf", case.stream), "first-run"), - ..Default::default() - }; - let _: CreateStreamResponse = call( - &case.ext, - "CreateStream", - CreateStreamRequest { - namespace: case.namespace.clone(), - owner: Some(workflow(&run)), - stream_ref: Some(chain.clone()), - ..Default::default() - }, - ) - .await - .unwrap(); - let close = CloseStreamRequest { - namespace: case.namespace.clone(), - r#ref: Some(chain), - writer: writer_for(workflow(&run)), - ..Default::default() - }; - let _: CloseStreamResponse = call(&case.ext, "CloseStream", close).await.unwrap(); - assert_eq!( - case.describe().await.unwrap().status, - StreamStatus::Closed as i32 - ); - } -} - -/// A Local Activity of a task that never completed is withdrawn by the run's next write. -async fn a_local_activity_of_a_task_that_never_completed_is_withdrawn_by_the_next_write() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - // Task [2] completed. Task [3] runs a Local Activity that writes, then task [3] fails. - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 2, 0, "done"), - ) - .await - .unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-2", 3, 2, "orphan"), - ) - .await - .unwrap(); - // Task [4] takes another path and writes once. - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-3", 4, 2, "other"), - ) - .await - .unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["done", "orphan", "", "other"]); - let superseded = superseded_of(&read.records[2]); - let kept: Vec<_> = [0, 1, 3] - .iter() - .filter(|&&i| !withdraws(superseded, &read.records[i])) - .map(|&i| texts(&read)[i].clone()) - .collect(); - assert_eq!(kept, ["done", "other"]); - // A late write of the orphaned call is fenced. - let late = call::<_, AppendRecordsResponse>( - &case.ext, - "AppendRecords", - AppendRecordsRequest { - sequence: 2, - ..local_activity_write(&case, &run, "call-2", 3, 2, "late") - }, - ) - .await - .unwrap_err(); - assert_eq!(reason(&late), "stream_fenced"); - // A later write that knows task [4] completed withdraws nothing more. - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-4", 7, 4, "later"), - ) - .await - .unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!( - texts(&read), - ["done", "orphan", "", "other", "later"] - ); - } -} - -/// A failed task before the last completed one is withdrawn by its id. -async fn a_failed_task_before_the_last_completed_one_is_withdrawn_by_its_id() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 3, 2, "orphan"), - ) - .await - .unwrap(); - // Task [3] failed and task [4] completed with no write here. Task [5] knows both. - let mut next = local_activity_write(&case, &run, "call-2", 5, 4, "next"); - withdrawal_mut(&mut next).failed_tasks = vec![3]; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", next).await.unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["orphan", "", "next"]); - let superseded = superseded_of(&read.records[1]); - assert!(withdraws(superseded, &read.records[0])); - assert!(!withdraws(superseded, &read.records[2])); - } -} - -/// A call of a task that failed before the call wrote is fenced on its first write, by the task -/// facts of the run's highest task. A write of a lower task keeps those facts. -async fn a_first_write_of_a_task_that_never_completed_is_fenced() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 2, 0, "done"), - ) - .await - .unwrap(); - // Task [3] runs call-2 and fails before call-2 writes. Task [4] knows that [3] failed. - let mut retry = local_activity_write(&case, &run, "call-3", 4, 2, "retry"); - withdrawal_mut(&mut retry).failed_tasks = vec![3]; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", retry).await.unwrap(); - // Call-1 of the completed task [2] still runs and writes with its older facts. - let more = AppendRecordsRequest { - sequence: 2, - ..local_activity_write(&case, &run, "call-1", 2, 0, "more") - }; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", more).await.unwrap(); - let late = call::<_, AppendRecordsResponse>( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-2", 3, 2, "late"), - ) - .await - .unwrap_err(); - assert_eq!(reason(&late), "stream_fenced"); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["done", "retry", "more"]); - } -} - -/// Task facts that a Store would drop or misread fail before any write. Only a Local Activity sends -/// them, failed tasks need the last completed task, and a started event id is positive. -async fn a_write_with_bad_task_facts_fails() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let facts = |failed: Vec| { - let mut request = local_activity_write(&case, &run, "call-1", 4, 2, "bad"); - withdrawal_mut(&mut request).failed_tasks = failed; - request - }; - let mut no_completed = facts(vec![3]); - withdrawal_mut(&mut no_completed).previous_completed_task = None; - let mut by_client = facts(vec![]); - writer_mut(&mut by_client).producer = Some(client(1)); - let mut by_activity = facts(vec![3]); - writer_mut(&mut by_activity).producer = Some(activity(1, 100)); - for bad in [ - facts(vec![0]), - facts(vec![-5]), - no_completed, - by_client, - by_activity, - ] { - let error = call::<_, AppendRecordsResponse>(&case.ext, "AppendRecords", bad) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_invalid"); - } - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert!(texts(&read).is_empty()); - } -} - -/// A Local Activity write with no task facts, as from an older Core, is withdrawn by a later write -/// whose facts show that its task never completed. -async fn a_write_with_no_task_facts_is_withdrawn_by_a_later_write() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let mut orphan = local_activity_write(&case, &run, "call-1", 3, 2, "orphan"); - withdrawal_mut(&mut orphan).previous_completed_task = None; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", orphan).await.unwrap(); - let mut next = local_activity_write(&case, &run, "call-2", 5, 4, "next"); - withdrawal_mut(&mut next).failed_tasks = vec![3]; - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", next).await.unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["orphan", "", "next"]); - let superseded = superseded_of(&read.records[1]); - assert!(withdraws(superseded, &read.records[0])); - assert!(!withdraws(superseded, &read.records[2])); - } -} - -/// A rerun of a call in a later task withdraws the failed task's Records with two items. The -/// orphan item has an entry of its own, and the newer epoch's item comes with the first Record. -async fn a_batch_writes_an_orphan_item_and_an_epoch_item() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 3, 2, "orphan"), - ) - .await - .unwrap(); - let mut rerun = local_activity_write(&case, &run, "call-1", 5, 4, "rerun"); - withdrawal_mut(&mut rerun).failed_tasks = vec![3]; - let answer: AppendRecordsResponse = call(&case.ext, "AppendRecords", rerun).await.unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!( - texts(&read), - ["orphan", "", "", "rerun"] - ); - let (orphans, epoch) = ( - superseded_of(&read.records[1]), - superseded_of(&read.records[2]), - ); - let kind = |item: &StreamSuperseded| -> Option { - Some( - item.writer - .as_ref()? - .producer - .as_ref()? - .kind - .as_ref()? - .clone(), - ) - }; - assert!(matches!(kind(orphans), Some(Kind::Workflow(_)))); - assert!(matches!(kind(epoch), Some(Kind::LocalActivity(_)))); - assert!(read.records[1].offset < answer.first_offset); - assert_eq!(read.records[2].offset, answer.first_offset); - assert_eq!(read.records[3].offset, answer.last_offset); - for item in [orphans, epoch] { - assert!(withdraws(item, &read.records[0])); - assert!(!withdraws(item, &read.records[3])); - } - } -} - -/// A resend of an empty batch that withdrew orphans gets the offsets of its orphan item again, and -/// adds no second item. -async fn a_resend_of_an_orphan_batch_withdraws_nothing_again() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let _: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - local_activity_write(&case, &run, "call-1", 3, 2, "orphan"), - ) - .await - .unwrap(); - // The writer of call-2 in task [4] opens with an empty batch. - let open = AppendRecordsRequest { - records: vec![], - ..local_activity_write(&case, &run, "call-2", 4, 2, "") - }; - let first: AppendRecordsResponse = call(&case.ext, "AppendRecords", open.clone()) - .await - .unwrap(); - let again: AppendRecordsResponse = call(&case.ext, "AppendRecords", open).await.unwrap(); - assert!(first.first_offset > 0); - assert_eq!( - (again.first_offset, again.last_offset), - (first.first_offset, first.last_offset) - ); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["orphan", ""]); - assert_eq!(read.records[1].offset, first.last_offset); - } -} - -/// The Workflow Task `[started_event_id]` in its attempt `attempt`, started at `seconds`. -fn task(started_event_id: i64, attempt: i32, seconds: i64) -> WorkflowTaskEpoch { - WorkflowTaskEpoch { - started_event_id, - attempt, - started_time: Some(Timestamp { seconds, nanos: 0 }), - } -} - -/// A write of the Local Activity `call` of `run`, dispatched by `task`, with the run's last -/// completed task, if the write sends it. -fn task_write( - case: &Case, - run: &str, - call: &str, - task: WorkflowTaskEpoch, - completed: Option, - text: &str, -) -> AppendRecordsRequest { - let producer = Producer { - id: crate::ids::local_activity_producer_id(run, call), - kind: Some(Kind::LocalActivity(LocalActivityProducer { - run_id: run.to_string(), - epoch: Some(LocalActivityEpoch { - task: Some(task), - attempt: 1, - }), - })), - }; - AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: Some(StreamWriter { - producer: Some(producer), - withdrawal: Some(RunWithdrawal { - previous_completed_task: completed, - ..Default::default() - }), - }), - sequence: 1, - records: vec![record(text)], - ..Default::default() - } -} - -/// A Local Activity of a transient attempt of a task that later completed is withdrawn by the -/// next write, which names the completed attempt. A Record of the completed attempt stays. -async fn a_local_activity_of_a_lower_attempt_of_the_completed_task_is_withdrawn() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let write = - |call: &str, - task: WorkflowTaskEpoch, - completed: Option, - text: &str| task_write(&case, &run, call, task, completed, text); - // Attempt 2 of task [9] writes, then fails with no event in History. - let c = Some(task(3, 1, 100)); - let orphan = write("call-1", task(9, 2, 200), c, "orphan"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", orphan).await.unwrap(); - // Attempt 3 completes. Its write sends no task facts, so it withdraws nothing itself. - let kept = write("call-2", task(9, 3, 300), None, "kept"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", kept).await.unwrap(); - let later = write("call-3", task(12, 1, 400), Some(task(9, 3, 300)), "later"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", later).await.unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["orphan", "kept", "", "later"]); - let superseded = superseded_of(&read.records[2]); - let kept: Vec<_> = [0, 1, 3] - .iter() - .filter(|&&i| !withdraws(superseded, &read.records[i])) - .map(|&i| texts(&read)[i].clone()) - .collect(); - assert_eq!(kept, ["kept", "later"]); - } -} - -/// A write sends the facts of its own task, attempt 3 of `[9]`. It withdraws the Records of -/// attempt 2 of `[9]`, which can't have completed, and keeps its own. -async fn a_write_withdraws_a_lower_attempt_of_its_own_task() { - for case in cases_for(Needs::UNCHECKED).await { - let run = format!("{}-run", case.stream); - case.create(None).await.unwrap(); - let completed = Some(task(3, 1, 100)); - let orphan = task_write(&case, &run, "call-1", task(9, 2, 200), completed, "orphan"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", orphan).await.unwrap(); - let own = task_write(&case, &run, "call-2", task(9, 3, 300), completed, "own"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", own).await.unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), ["orphan", "", "own"]); - let superseded = superseded_of(&read.records[1]); - assert!(withdraws(superseded, &read.records[0])); - assert!(!withdraws(superseded, &read.records[2])); - } -} - -/// A run that a retry run withdrew can't close the Stream, and the retry run can. -async fn a_withdrawn_run_cant_close_the_stream() { - for case in cases_for(Needs::UNCHECKED).await { - let (previous, retry) = (format!("{}-r1", case.stream), format!("{}-r2", case.stream)); - let create = |run: &str, withdraw_run: &str| CreateStreamRequest { - namespace: case.namespace.clone(), - withdraw_run: withdraw_run.to_string(), - owner: Some(workflow(run)), - stream_ref: owned_ref(&case), - ..use_existing() - }; - let close = |run: &str| CloseStreamRequest { - namespace: case.namespace.clone(), - r#ref: owned_ref(&case), - writer: writer_for(workflow(run)), - ..Default::default() - }; - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&previous, "")) - .await - .unwrap(); - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&retry, &previous)) - .await - .unwrap(); - let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", close(&previous)) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_fenced"); - let _: CloseStreamResponse = call(&case.ext, "CloseStream", close(&retry)).await.unwrap(); - } -} - -/// A retry run's create of a new Stream fences the previous run, whose later write fails. -async fn a_retry_run_s_create_of_a_new_stream_fences_the_previous_run() { - for case in cases_for(Needs::UNCHECKED).await { - let (previous, retry) = (format!("{}-r1", case.stream), format!("{}-r2", case.stream)); - let created: CreateStreamResponse = call( - &case.ext, - "CreateStream", - CreateStreamRequest { - namespace: case.namespace.clone(), - withdraw_run: previous.clone(), - owner: Some(workflow(&retry)), - stream_ref: owned_ref(&case), - ..Default::default() - }, - ) - .await - .unwrap(); - assert!(created.created); - let append = |run: &str| AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: writer_for(local_activity(run)), - sequence: 1, - records: vec![record(run)], - ..Default::default() - }; - let error = call::<_, AppendRecordsResponse>(&case.ext, "AppendRecords", append(&previous)) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_fenced"); - let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", append(&retry)) - .await - .unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - assert_eq!(texts(&read), [retry]); - } -} - -/// A retry run's create after the log expired keeps the gap, so a read from an offset that the -/// log took fails as expired. -async fn a_retry_run_s_create_after_the_log_expired_keeps_the_gap() { - for case in cases_for(Needs::UNCHECKED.and(Needs::SHORT_TIMES)).await { - let (previous, retry) = (format!("{}-r1", case.stream), format!("{}-r2", case.stream)); - let create = |run: &str, withdraw_run: &str| CreateStreamRequest { - namespace: case.namespace.clone(), - retention: millis(300), - withdraw_run: withdraw_run.to_string(), - owner: Some(workflow(run)), - stream_ref: owned_ref(&case), - ..use_existing() - }; - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&previous, "")) - .await - .unwrap(); - let first: AppendRecordsResponse = call( - &case.ext, - "AppendRecords", - AppendRecordsRequest { - namespace: case.namespace.clone(), - r#ref: case.stream_ref(), - writer: writer_for(local_activity(&previous)), - sequence: 1, - records: vec![record("a"), record("b")], - ..Default::default() - }, - ) - .await - .unwrap(); - tokio::time::sleep(Duration::from_millis(400)).await; - let _: CreateStreamResponse = call(&case.ext, "CreateStream", create(&retry, &previous)) - .await - .unwrap(); - let behind = ReadRecordsRequest { - from_offset: first.first_offset + 1, - ..Default::default() - }; - let error = case.read(behind).await.unwrap_err(); - assert_eq!(reason(&error), "stream_expired"); - } -} - /// A read of a closed Stream gives `done` only with its last item. With no item left, a read /// answers at once with `done`, even with a long wait. async fn a_read_of_a_closed_stream_with_no_item_left_answers_at_once() { @@ -1802,7 +804,7 @@ async fn a_read_of_a_closed_stream_with_no_item_left_answers_at_once() { }) .await .unwrap(); - assert_eq!(texts(&rest), ["2"]); + assert_eq!(texts(&rest), ["2", ""]); assert!(rest.done); for request in [ ReadRecordsRequest { @@ -1860,9 +862,8 @@ async fn a_waiting_read_wakes_with_done_on_a_close() { } } -/// A last read gives the newest Records oldest first, skips withdrawn ones, and gives `done` for a -/// closed Stream. -async fn a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_ones() { +/// A last read gives the newest Records oldest first, and gives `done` for a closed Stream. +async fn a_last_read_gives_the_newest_records_oldest_first() { for case in cases_for(Needs::CALLS).await { case.create(None).await.unwrap(); let last = |n: i32| ReadRecordsRequest { @@ -1872,19 +873,9 @@ async fn a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_one assert!(case.read(last(5)).await.unwrap().records.is_empty()); let records = vec![record("c1"), record("c2"), record("c3")]; case.append(client(1), 1, records).await.unwrap(); - if case.gives.gives(Needs::UNCHECKED) { - case.append(activity(1, 10), 1, vec![record("a1")]) - .await - .unwrap(); - // The retry's first batch withdraws a1, so a1 doesn't count. - case.append(activity(2, 20), 1, vec![record("a2")]) - .await - .unwrap(); - } else { - case.append(named("w", 1), 1, vec![record("a2")]) - .await - .unwrap(); - } + case.append(named("w", 1), 1, vec![record("a2")]) + .await + .unwrap(); assert_eq!( texts(&case.read(last(3)).await.unwrap()), ["c2", "c3", "a2"] @@ -1936,41 +927,21 @@ pub const CASES: &[super::Case] = &[ case!(a_create_of_an_open_stream_fails_unless_it_uses_it_or_retries), case!(a_create_of_a_closed_stream_fails), case!(a_close_writes_no_record_and_a_second_close_succeeds), - case!(a_workflow_or_a_client_closes_a_stream_with_no_owner), case!(a_create_of_an_expired_stream_fails), - case!(a_create_checks_its_owner_and_its_ref), case!(a_producer_must_match_its_kind_and_not_be_a_workflow), case!(an_append_with_no_sequence_takes_the_next_one_of_its_epoch), case!(a_read_is_inclusive_from_its_offset_and_get_offset_reads_only_new_records), - case!(a_higher_attempt_puts_its_superseded_item_before_its_batch), case!(a_read_waits_for_the_create_only_as_long_as_create_wait), case!(a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append), case!(a_close_ends_every_read_with_done), + case!(a_closed_read_ends_with_a_close_item), case!(an_unknown_method_is_unimplemented), - case!(a_read_of_an_expired_open_stream_fails_instead_of_waiting), case!(a_read_of_an_expired_closed_stream_fails_instead_of_waiting), case!(a_read_behind_an_expired_log_fails_before_and_after_the_next_write), case!(a_close_after_the_log_expired_keeps_the_gap), - case!(a_ref_with_an_owner_and_one_without_reach_the_same_stream), case!(a_create_answer_names_the_backend_and_the_store_never_reads_it), - case!(a_retry_run_s_local_activity_calls_withdraw_the_previous_run), - case!(only_a_later_run_s_local_activity_withdraws_a_run_on_append), - case!(a_rerun_local_activity_supersedes_its_first_attempt), - case!(the_close_activity_closes_a_workflow_stream_as_its_workflow_producer), - case!(a_local_activity_of_a_task_that_never_completed_is_withdrawn_by_the_next_write), - case!(a_failed_task_before_the_last_completed_one_is_withdrawn_by_its_id), - case!(a_first_write_of_a_task_that_never_completed_is_fenced), - case!(a_write_with_bad_task_facts_fails), - case!(a_write_with_no_task_facts_is_withdrawn_by_a_later_write), - case!(a_batch_writes_an_orphan_item_and_an_epoch_item), - case!(a_resend_of_an_orphan_batch_withdraws_nothing_again), - case!(a_local_activity_of_a_lower_attempt_of_the_completed_task_is_withdrawn), - case!(a_write_withdraws_a_lower_attempt_of_its_own_task), - case!(a_withdrawn_run_cant_close_the_stream), - case!(a_retry_run_s_create_of_a_new_stream_fences_the_previous_run), - case!(a_retry_run_s_create_after_the_log_expired_keeps_the_gap), case!(a_read_of_a_closed_stream_with_no_item_left_answers_at_once), case!(a_waiting_read_wakes_with_done_on_a_close), - case!(a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_ones), + case!(a_last_read_gives_the_newest_records_oldest_first), case!(a_page_keeps_under_max_bytes_and_a_larger_record_comes_alone), ]; diff --git a/crates/streams/src/testing/store.rs b/crates/streams/src/testing/store.rs index 9221007d4..27fde6818 100644 --- a/crates/streams/src/testing/store.rs +++ b/crates/streams/src/testing/store.rs @@ -4,24 +4,18 @@ use super::{ case, fields, - support::{ - Needs, Run, Target, digest, hex, record, records_of, stores_for, superseded, - workflow_execution, - }, + support::{Needs, Run, Target, digest, record, records_of, stores_for}, }; use crate::{ Reason, StreamResult, epoch::{ProducerKind, epoch_key}, - ids::{ - activity_producer_id, client_producer_id, named_client_producer_id, workflow_producer_id, - }, + ids::{activity_producer_id, client_producer_id, named_client_producer_id}, memory::{EntryId, ProducerField}, offset::{entry_id_of_offset, offset_of_entry_id}, proto::{ - ActivityEpoch, ActivityProducer, ClientEpoch, ClientProducer, Producer, StreamRef, - WorkflowProducer, WorkflowTaskEpoch, producer::Kind, + ActivityEpoch, ActivityProducer, ClientEpoch, ClientProducer, Producer, producer::Kind, }, - store::{Batch, ConflictPolicy, Create, CreateOutcome, Created, Owner, TaskFacts, Withdrawal}, + store::{Batch, ConflictPolicy, Create, CreateOutcome, Created}, }; use futures_util::future::join_all; use prost_types::Timestamp; @@ -67,9 +61,6 @@ async fn append( texts: &[&str], ) -> Result { let records: Vec<_> = texts.iter().map(|text| record(producer, text)).collect(); - let mut withdrawal = Withdrawal::default(); - let bytes = superseded(producer, &withdrawal); - withdrawal.superseded = &bytes; target .store .append( @@ -80,7 +71,6 @@ async fn append( sequence, records: &records, digest: digest(texts), - withdrawal, }, ) .await @@ -171,10 +161,9 @@ async fn a_create_of_an_open_stream_fails_unless_it_uses_it_or_retries() { } } -/// A create of an expired, another owner's or a closed Stream fails as `expired`, `not_owner` or -/// `closed`, in this order. These come before the conflict policy and before a retry of the create -/// that made the Stream. -async fn closed_expired_and_not_owner_come_before_already_exists() { +/// A create of an expired or a closed Stream fails as `expired` or `closed`, in this order. These +/// come before the conflict policy and before a retry of the create that made the Stream. +async fn closed_and_expired_come_before_already_exists() { for target in stores_for(Needs::RAW_KEYS).await { let first = Create { request_id: "create-1", @@ -182,23 +171,11 @@ async fn closed_expired_and_not_owner_come_before_already_exists() { }; target.create_with(&first).await.unwrap(); let reason = |created: StreamResult| created.unwrap_err().reason; - let owned = StreamRef { - owner: workflow_execution("agent", "first-run"), - ..target.stream_ref() - }; - let workflow = Create { - owner: Owner::Workflow, - ..first.clone() - }; - let not_owner = || target.store.create(&target.namespace, &owned, &workflow); - assert_eq!(reason(not_owner().await), Reason::NotOwner); target.close(&client(1)).await.unwrap(); - assert_eq!(reason(not_owner().await), Reason::NotOwner); for create in [&first, &Create::default()] { assert_eq!(reason(target.create_with(create).await), Reason::Closed); } target.set_meta("exp", "1").await; - assert_eq!(reason(not_owner().await), Reason::Expired); assert_eq!(reason(target.create_with(&first).await), Reason::Expired); } } @@ -207,7 +184,7 @@ async fn closed_expired_and_not_owner_come_before_already_exists() { /// Producer wrote, and a lower sequence is refused. async fn a_resend_gets_its_first_positions_and_a_lower_sequence_is_refused() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); let first = append(&target, &p, 1, &["p1", "p2"]).await.unwrap(); // Another Producer writes between the batch and its resend. @@ -230,7 +207,7 @@ async fn a_resend_gets_its_first_positions_and_a_lower_sequence_is_refused() { /// A resend with the last sequence and other content fails as `seq_divergent` and writes nothing. async fn a_resend_with_other_content_fails_as_divergent() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); append(&target, &p, 1, &["p1"]).await.unwrap(); let divergent = append(&target, &p, 1, &["other"]).await.unwrap_err(); @@ -242,7 +219,7 @@ async fn a_resend_with_other_content_fails_as_divergent() { /// A lower epoch is fenced for each Producer kind. async fn a_lower_epoch_is_fenced_for_each_producer_kind() { for target in stores_for(Needs::UNCHECKED).await { - target.create(Owner::Workflow).await; + target.create().await; let run = Run::new("r1"); let la = run.local_activity("k", 8, 1, 2); target.batch(&la, 1, &["la2"]).await.unwrap(); @@ -266,7 +243,7 @@ async fn a_lower_epoch_is_fenced_for_each_producer_kind() { /// fails as closed. With `STORE_ANSWERS`, a second close returns `already`. async fn a_close_stops_appends_but_not_a_resend() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); let first = append(&target, &p, 1, &["a"]).await.unwrap(); let closed = target.close(&p).await.unwrap(); @@ -291,7 +268,7 @@ async fn a_close_stops_appends_but_not_a_resend() { /// reads the Producer's `p|` field, since a newer format can change that field. async fn a_newer_format_refuses_every_write() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); append(&target, &p, 1, &["a"]).await.unwrap(); target.set_meta(fields::VERSION, "2").await; @@ -311,7 +288,7 @@ async fn a_newer_format_refuses_every_write() { /// An expired Stream refuses writes and its create. async fn an_expired_stream_refuses_writes_and_its_create() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; target.set_meta("exp", "1").await; let error = append(&target, &client(1), 1, &["a"]).await.unwrap_err(); assert_eq!(error.reason, Reason::Expired); @@ -320,113 +297,12 @@ async fn an_expired_stream_refuses_writes_and_its_create() { } } -/// An Activity retry flags only its first batch, which withdraws the earlier attempt. -async fn an_activity_retry_flags_its_first_batch_only() { - for target in stores_for(Needs::UNCHECKED.and(Needs::STORE_ANSWERS)).await { - target.create(Owner::Workflow).await; - let (a1, a2) = (activity("r1", 1, 10), activity("r1", 2, 20)); - let other = Producer { - id: activity_producer_id("r1", "b", "s"), - ..activity("r1", 1, 10) - }; - append(&target, &a1, 1, &["x1"]).await.unwrap(); - append(&target, &other, 1, &["y1"]).await.unwrap(); - assert!(append(&target, &a2, 1, &["x2"]).await.unwrap().flagged); - assert!(!append(&target, &a2, 2, &["x3"]).await.unwrap().flagged); - assert_eq!(target.visible().await, ["y1", "x2", "x3"]); - } -} - -/// An empty batch of an Activity retry withdraws the earlier attempt's Records. -async fn an_empty_batch_of_a_retry_withdraws_the_earlier_attempt() { - for target in stores_for(Needs::UNCHECKED).await { - target.create(Owner::Workflow).await; - append(&target, &activity("r1", 1, 10), 1, &["x1"]) - .await - .unwrap(); - let empty = append(&target, &activity("r1", 2, 20), 1, &[]) - .await - .unwrap(); - assert!(empty.flagged); - assert!(empty.offsets.is_some()); - assert!(target.visible().await.is_empty()); - // The withdrawal is done, so another empty batch writes nothing. - let again = append(&target, &activity("r1", 3, 30), 1, &[]) - .await - .unwrap(); - assert!(!again.flagged); - assert_eq!(again.offsets, None); - // An empty batch still raises the fence. - let stale = append(&target, &activity("r1", 2, 20), 2, &["late"]).await; - assert_eq!(stale.unwrap_err().reason, Reason::Fenced); - } -} - -/// A retry run's first write withdraws the previous run and fences its Producers. -async fn a_retry_run_withdraws_and_fences_the_previous_run() { - for target in stores_for(Needs::UNCHECKED).await { - target.create(Owner::Workflow).await; - let (r1, r2) = (Run::new("r1"), Run::new("r2")); - target - .batch(&r1.local_activity("k", 3, 1, 1), 1, &["A"]) - .await - .unwrap(); - append(&target, &activity("r1", 1, 10), 1, &["A'"]) - .await - .unwrap(); - let b = target - .batch_retry(&r2.local_activity("k", 3, 1, 1), "r1", &["B"]) - .await - .unwrap(); - assert!(b.flagged); - assert_eq!(target.visible().await, ["B"]); - let late = append(&target, &activity("r1", 1, 10), 2, &["A''"]).await; - assert_eq!(late.unwrap_err().reason, Reason::Fenced); - let late = target - .batch(&r1.local_activity("j", 6, 1, 1), 1, &["A3"]) - .await; - assert_eq!(late.unwrap_err().reason, Reason::Fenced); - // A second withdrawal of the same run withdraws nothing more. - let again = target - .batch_retry(&r2.local_activity("j", 6, 1, 1), "r1", &["B2"]) - .await - .unwrap(); - assert!(!again.flagged); - } -} - -/// An empty batch that withdraws a run keeps its Producer's held epoch, whose Records it doesn't -/// withdraw. So the Producer's next epoch still withdraws them. -async fn an_empty_batch_that_withdraws_a_run_keeps_the_held_epoch() { - for target in stores_for(Needs::UNCHECKED).await { - target.create(Owner::Workflow).await; - let (r1, r2) = (Run::new("r1"), Run::new("r2")); - target - .batch(&r1.local_activity("k", 3, 1, 1), 1, &["A"]) - .await - .unwrap(); - let first = r2.local_activity("k", 3, 1, 1); - target.batch(&first, 1, &["B1"]).await.unwrap(); - target.write(&first, 2, "r1", &[]).await.unwrap(); - assert_eq!(target.visible().await, ["B1"]); - target - .batch(&r2.local_activity("k", 3, 1, 2), 1, &["B2"]) - .await - .unwrap(); - assert_eq!(target.visible().await, ["B2"]); - } -} - -/// An open Workflow Stream's meta keeps the safety TTL, and its log keeps the Retention, after -/// which a read hides every Record. Its close gives the meta the Retention too. -async fn the_ttl_follows_the_owner_and_the_close() { +/// An open Stream's meta keeps the safety TTL, and its log keeps the Retention, after which a read +/// hides every Record. Its close gives the meta the Retention too. +async fn the_ttl_is_the_safety_ttl_until_the_close() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Workflow).await; - let run = Run::new("r1"); - target - .batch(&run.local_activity("k", 3, 1, 1), 1, &["a"]) - .await - .unwrap(); + target.create().await; + append(&target, &client(1), 1, &["a"]).await.unwrap(); let week = Duration::from_secs(7 * 86_400).as_millis() as i64; let (ttl, meta) = target.ttls().await; assert!(0 < ttl && ttl <= week, "{ttl}"); @@ -434,7 +310,7 @@ async fn the_ttl_follows_the_owner_and_the_close() { meta > Duration::from_secs(80 * 86_400).as_millis() as i64, "{meta}" ); - target.close(&workflow("r1")).await.unwrap(); + target.close(&client(1)).await.unwrap(); let (ttl, meta) = target.ttls().await; assert!(0 < ttl && ttl <= week, "{ttl}"); assert!( @@ -451,7 +327,6 @@ async fn an_append_trims_by_retention_and_keeps_the_watermark() { target .create_with(&Create { retention: Some(Duration::from_millis(600)), - owner: Owner::Workflow, ..Default::default() }) .await @@ -487,7 +362,6 @@ async fn offsets_go_on_after_the_log_expires() { target .create_with(&Create { retention: Some(Duration::from_millis(200)), - owner: Owner::Workflow, ..Default::default() }) .await @@ -518,7 +392,7 @@ async fn offsets_go_on_after_the_log_expires() { /// A wait for the next entry wakes on an append and ends at its deadline. async fn a_blocked_read_wakes_on_an_append_and_ends_at_its_deadline() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; let first = append(&target, &client(1), 1, &["a"]).await.unwrap(); let newest = first.offsets.unwrap().1; let deadline = tokio::time::Instant::now() + Duration::from_millis(200); @@ -546,7 +420,7 @@ async fn a_blocked_read_wakes_on_an_append_and_ends_at_its_deadline() { /// ends at once, so a waiting read sees the close. async fn a_close_writes_no_entry_and_ends_a_wait() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; let first = append(&target, &client(1), 1, &["a"]).await.unwrap(); let newest = first.offsets.unwrap().1; let store = target.store.clone(); @@ -582,7 +456,7 @@ async fn a_close_writes_no_entry_and_ends_a_wait() { /// caller knows the offset of each one. async fn a_batch_gets_consecutive_offsets_that_end_at_its_last_offset() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; append(&target, &client(1), 1, &["before"]).await.unwrap(); // Enough Records that `XADD *` on Redis 7.0 would cross a millisecond. let texts: Vec = (0..1000).map(|i| i.to_string()).collect(); @@ -607,7 +481,7 @@ async fn a_batch_gets_consecutive_offsets_that_end_at_its_last_offset() { /// A negative sequence is refused. async fn a_negative_sequence_is_refused() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let error = append(&target, &client(1), -1, &["x"]).await.unwrap_err(); assert_eq!(error.reason, Reason::Invalid); assert!(target.texts().await.is_empty()); @@ -619,7 +493,7 @@ async fn a_negative_sequence_is_refused() { /// number is a resend. async fn a_batch_with_no_sequence_takes_the_next_one_of_its_epoch() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); append(&target, &p, 0, &["a"]).await.unwrap(); let retry = append(&target, &p, 0, &["a"]).await.unwrap(); @@ -643,7 +517,7 @@ async fn a_batch_with_no_sequence_takes_the_next_one_of_its_epoch() { /// the Store writes all of them. async fn batches_with_no_sequence_at_once_each_take_their_own_number() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); let texts = ["t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8"]; let appends = texts @@ -665,7 +539,7 @@ async fn batches_with_no_sequence_at_once_each_take_their_own_number() { /// divergent batch is refused. A sequence past 2^53 - 1 is refused. async fn a_large_sequence_dedupes_and_one_past_2_53_is_refused() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); let large = 100_000_000_000_000; let first = append(&target, &p, large, &["x"]).await.unwrap(); @@ -683,55 +557,6 @@ async fn a_large_sequence_dedupes_and_one_past_2_53_is_refused() { } } -/// A create that names another owner than the Stream has fails and changes nothing. So a Workflow -/// doesn't take a Stream with no owner, and another run chain of a Workflow id is another owner. -async fn a_create_with_another_owner_is_refused() { - for target in stores_for(Needs::RAW_KEYS.and(Needs::STORE_ANSWERS)).await { - let target = ⌖ - let create = |stream_ref: StreamRef, owner: Owner| async move { - let create = Create { - owner, - ..use_existing() - }; - target - .store - .create(&target.namespace, &stream_ref, &create) - .await - }; - let owned = |workflow_id: &str, first_run_id: &str| StreamRef { - owner: workflow_execution(workflow_id, first_run_id), - ..target.stream_ref() - }; - target.create(Owner::Client).await; - let refused = create(owned("agent", "first-run"), Owner::Workflow).await; - assert_eq!(refused.unwrap_err().reason, Reason::NotOwner); - assert_eq!(target.meta("own").await.as_deref(), Some(&b"client"[..])); - // The meta keeps the TTL of a client's Stream, below the safety TTL of 90 days. - let (_, ttl) = target.ttls().await; - assert!( - ttl < Duration::from_secs(90 * 86_400).as_millis() as i64, - "{ttl}" - ); - - let owned = |workflow_id: &str, first_run_id: &str| StreamRef { - id: format!("{}-owned", target.stream), - ..owned(workflow_id, first_run_id) - }; - let created = create(owned("agent", "first-run"), Owner::Workflow).await; - assert_eq!(created.unwrap().outcome, CreateOutcome::Created); - for (stream, owner) in [ - (owned("agent", "other-run"), Owner::Workflow), - (owned("another-agent", "first-run"), Owner::Workflow), - (owned("agent", "first-run"), Owner::Client), - ] { - let refused = create(stream, owner).await; - assert_eq!(refused.unwrap_err().reason, Reason::NotOwner); - } - let again = create(owned("agent", "first-run"), Owner::Workflow).await; - assert_eq!(again.unwrap().outcome, CreateOutcome::Exists); - } -} - /// An append removes the `p|` field of a Producer whose last append is older than the Retention, /// since its Records are gone too. A lower epoch of that Producer then writes. async fn an_append_forgets_the_producers_that_the_retention_outlived() { @@ -739,7 +564,6 @@ async fn an_append_forgets_the_producers_that_the_retention_outlived() { target .create_with(&Create { retention: Some(Duration::from_millis(300)), - owner: Owner::Workflow, ..Default::default() }) .await @@ -759,7 +583,7 @@ async fn an_append_forgets_the_producers_that_the_retention_outlived() { /// A Producer with a lower epoch can't close the Stream. async fn a_stale_producer_cant_close_the_stream() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; append(&target, &client(2), 1, &["a"]).await.unwrap(); assert_eq!( target.close(&client(1)).await.unwrap_err().reason, @@ -769,36 +593,11 @@ async fn a_stale_producer_cant_close_the_stream() { } } -/// The close Activity's Producer of run `run`. -fn workflow(run: &str) -> Producer { - Producer { - id: workflow_producer_id(run), - kind: Some(Kind::Workflow(WorkflowProducer { - run_id: run.to_string(), - epoch: Some(WorkflowTaskEpoch::default()), - })), - } -} - -/// A Stream with no owner takes a close from a Workflow or a client. A Local Activity or an -/// Activity can't close it. -async fn a_workflow_or_a_client_closes_a_stream_with_no_owner() { - for target in stores_for(Needs::UNCHECKED).await { - target.create(Owner::Client).await; - let la = Run::new("r1").local_activity("k", 3, 1, 1); - for producer in [&la, &activity("r1", 1, 10)] { - let refused = target.close(producer).await.unwrap_err(); - assert_eq!(refused.reason, Reason::NotOwner); - } - target.close(&workflow("r1")).await.unwrap(); - } -} - /// A Store refuses an append or a close of a Producer with no epoch, or with an epoch that has no /// case, since the case gives the kind of the Producer. async fn a_producer_with_no_epoch_case_is_refused() { for target in stores_for(Needs::CALLS).await { - target.create(Owner::Client).await; + target.create().await; let producer = Producer { kind: None, ..client(1) @@ -811,56 +610,10 @@ async fn a_producer_with_no_epoch_case_is_refused() { } } -/// Only the owner closes a Stream. That's the close Activity of its run chain for a Workflow-owned -/// Stream, and a Workflow or a client for a Stream with no owner. A Local Activity of the chain -/// can't close it, and a ref with no chain can't either. -async fn only_the_owner_closes_a_stream() { - for target in stores_for(Needs::UNCHECKED.and(Needs::STORE_ANSWERS)).await { - let run = Run::new("r1"); - let la = run.local_activity("k", 3, 1, 1); - target.create(Owner::Client).await; - for producer in [&la, &activity("r1", 1, 10)] { - let refused = target.close(producer).await.unwrap_err(); - assert_eq!(refused.reason, Reason::NotOwner); - } - target.close(&client(1)).await.unwrap(); - - let owned = |workflow_id: &str| StreamRef { - id: format!("{}-owned", target.stream), - owner: workflow_execution(workflow_id, "first-run").filter(|_| !workflow_id.is_empty()), - ..Default::default() - }; - let create = Create { - owner: Owner::Workflow, - ..Default::default() - }; - target - .store - .create(&target.namespace, &owned("agent"), &create) - .await - .unwrap(); - for (stream, producer) in [ - (owned("agent"), client(1)), - (owned("agent"), activity("r1", 1, 10)), - (owned("agent"), la.clone()), - (owned("another-agent"), workflow("r1")), - (owned(""), workflow("r1")), - ] { - let refused = target.close_at(&stream, &producer).await.unwrap_err(); - assert_eq!(refused.reason, Reason::NotOwner); - } - let closed = target.close_at(&owned("agent"), &workflow("r1")).await; - assert!(!closed.unwrap().already); - // The owner check comes first, even on a closed Stream. - let refused = target.close_at(&owned("agent"), &la).await.unwrap_err(); - assert_eq!(refused.reason, Reason::NotOwner); - } -} - /// An append whose entry ids would not fit an offset is refused before it writes. async fn an_append_whose_ids_would_not_fit_an_offset_is_refused_before_it_writes() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; // An entry a minute ahead of the clock, with nearly the last 20-bit sequence. let ahead = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -876,90 +629,6 @@ async fn an_append_whose_ids_would_not_fit_an_offset_is_refused_before_it_writes } } -/// Every Store keeps the meta fields that the shared vectors name. -async fn every_store_keeps_the_meta_fields_of_the_shared_vectors() { - let vectors: serde_json::Value = - serde_json::from_str(include_str!("../../testdata/fields.json")).unwrap(); - for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Workflow).await; - let producer = activity("run:1", 1, 10); - append(&target, &producer, 1, &["a"]).await.unwrap(); - let kind = ProducerKind::Activity as i32; - // The vector's Producer is this one, so its field names are the ones the Store keeps. - assert_eq!(producer.id, vectors["producer_id"].as_str().unwrap()); - let field = fields::producer(kind, &producer.id); - assert_eq!(field, vectors["producer"].as_str().unwrap()); - for field in [ - field, - vectors["run_wrote"].as_str().unwrap().to_string(), - fields::VERSION.to_string(), - ] { - assert!(target.meta(&field).await.is_some(), "{field}"); - } - // Task [3] fails, and a write of task [5] with the completed task [4] withdraws it. - let run = Run::new("run:1"); - task_batch(&target, &run.local_activity("k", 3, 1, 1), 2, &[], &["b"]) - .await - .unwrap(); - task_batch(&target, &run.local_activity("j", 5, 1, 1), 4, &[3], &["c"]) - .await - .unwrap(); - let text = |field: &str| vectors[field].as_str().unwrap(); - let withdrawn = [ - text("withdrawn_task_prefix").as_bytes(), - &hex(text("task_key")), - ] - .concat(); - assert!(target.meta(text("open_tasks")).await.is_some()); - assert!(target.meta(withdrawn).await.is_some()); - assert_eq!( - target.meta(text("task_facts")).await, - Some(hex(text("task_facts_value"))) - ); - } -} - -/// A batch of a Local Activity with its run's task facts, the run's completed task -/// `[completed]` and its failed tasks. -async fn task_batch( - target: &Target, - producer: &Producer, - completed: i64, - failed_tasks: &[i64], - texts: &[&str], -) -> Result { - let records: Vec<_> = texts.iter().map(|text| record(producer, text)).collect(); - let completed = WorkflowTaskEpoch { - started_event_id: completed, - ..Default::default() - }; - // The Store keeps the `Superseded` facts as sent, so these do for both items. - let bytes = superseded(producer, &Withdrawal::default()); - let withdrawal = Withdrawal { - withdraw_run: "", - superseded: &bytes, - tasks: Some(TaskFacts { - completed: Some(&completed), - failed_tasks, - superseded: &bytes, - }), - }; - target - .store - .append( - &target.namespace, - &target.stream_ref(), - &Batch { - producer, - sequence: 1, - records: &records, - digest: digest(texts), - withdrawal, - }, - ) - .await -} - /// The entry ids of an append's offsets. fn entry_ids(offsets: Option<(i64, i64)>) -> Option<(EntryId, EntryId)> { let id = |offset: i64| { @@ -970,17 +639,15 @@ fn entry_ids(offsets: Option<(i64, i64)>) -> Option<(EntryId, EntryId)> { offsets.map(|(first, last)| (id(first), id(last))) } -/// A Producer's `p|` field has the same 129 bytes on every Store. These are the flags, the fence, -/// the held epoch, and the last batch's sequence, entry ids and digest. +/// A Producer's `p|` field has the same 88 bytes on every Store. These are the fence, and the last +/// batch's sequence, entry ids and digest. async fn a_producer_field_has_the_shared_layout() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Workflow).await; + target.create().await; let kind = ProducerKind::Activity as i32; - let field = |producer: &Producer, held: Option<&Producer>, offsets, texts: &[&str]| { - let epoch = |producer: &Producer| epoch_key(producer.kind.as_ref()); + let field = |producer: &Producer, offsets, texts: &[&str]| { ProducerField { - fence: epoch(producer), - held: held.map(epoch), + fence: epoch_key(producer.kind.as_ref()), sequence: 1, entries: entry_ids(offsets), digest: digest(texts), @@ -990,34 +657,26 @@ async fn a_producer_field_has_the_shared_layout() { let a1 = activity("r1", 1, 10); let stored = || async { target.meta(fields::producer(kind, &a1.id)).await.unwrap() }; let written = append(&target, &a1, 1, &["x", "y"]).await.unwrap(); - assert_eq!( - stored().await, - field(&a1, Some(&a1), written.offsets, &["x", "y"]) - ); - // A newer attempt's empty batch withdraws the held epoch's Records, so none is held. + assert_eq!(stored().await, field(&a1, written.offsets, &["x", "y"])); + // A newer attempt's empty batch sets its fence but writes no entry. let a2 = activity("r1", 2, 20); - let flagged = append(&target, &a2, 1, &[]).await.unwrap(); - assert_eq!(stored().await, field(&a2, None, flagged.offsets, &[])); - // Nothing is left to withdraw, so the next empty batch writes no entry. - let a3 = activity("r1", 3, 30); - let empty = append(&target, &a3, 1, &[]).await.unwrap(); + let empty = append(&target, &a2, 1, &[]).await.unwrap(); assert_eq!(empty.offsets, None); - assert_eq!(stored().await, field(&a3, None, None, &[])); + assert_eq!(stored().await, field(&a2, None, &[])); } } -/// A Store refuses a write of a Producer whose `p|` field has another length or another flag bit, -/// which a later format may write. +/// A Store refuses a write of a Producer whose `p|` field has another length, which a later format +/// may write. async fn a_producer_field_of_another_format_is_refused() { for target in stores_for(Needs::RAW_KEYS).await { - target.create(Owner::Client).await; + target.create().await; let p = client(1); append(&target, &p, 1, &["a"]).await.unwrap(); let field = fields::producer(ProducerKind::Client as i32, &p.id); let value = target.meta(&field).await.unwrap(); - let mut flagged = value.clone(); - flagged[0] |= 2; - for other in [flagged, value[..value.len() - 1].to_vec()] { + let longer = [value.clone(), vec![0]].concat(); + for other in [longer, value[..value.len() - 1].to_vec()] { target.set_meta(&field, &other).await; let error = append(&target, &p, 2, &["b"]).await.unwrap_err(); assert_eq!(error.reason, Reason::StorageCorrupt); @@ -1032,18 +691,14 @@ async fn a_producer_field_of_another_format_is_refused() { pub const CASES: &[super::Case] = &[ case!(a_create_writes_the_key_once_and_keeps_its_retention), case!(a_create_of_an_open_stream_fails_unless_it_uses_it_or_retries), - case!(closed_expired_and_not_owner_come_before_already_exists), + case!(closed_and_expired_come_before_already_exists), case!(a_resend_gets_its_first_positions_and_a_lower_sequence_is_refused), case!(a_resend_with_other_content_fails_as_divergent), case!(a_lower_epoch_is_fenced_for_each_producer_kind), case!(a_close_stops_appends_but_not_a_resend), case!(a_newer_format_refuses_every_write), case!(an_expired_stream_refuses_writes_and_its_create), - case!(an_activity_retry_flags_its_first_batch_only), - case!(an_empty_batch_of_a_retry_withdraws_the_earlier_attempt), - case!(a_retry_run_withdraws_and_fences_the_previous_run), - case!(an_empty_batch_that_withdraws_a_run_keeps_the_held_epoch), - case!(the_ttl_follows_the_owner_and_the_close), + case!(the_ttl_is_the_safety_ttl_until_the_close), case!(an_append_trims_by_retention_and_keeps_the_watermark), case!(offsets_go_on_after_the_log_expires), case!(a_blocked_read_wakes_on_an_append_and_ends_at_its_deadline), @@ -1053,14 +708,10 @@ pub const CASES: &[super::Case] = &[ case!(a_batch_with_no_sequence_takes_the_next_one_of_its_epoch), case!(batches_with_no_sequence_at_once_each_take_their_own_number), case!(a_large_sequence_dedupes_and_one_past_2_53_is_refused), - case!(a_create_with_another_owner_is_refused), case!(an_append_forgets_the_producers_that_the_retention_outlived), case!(a_stale_producer_cant_close_the_stream), - case!(a_workflow_or_a_client_closes_a_stream_with_no_owner), case!(a_producer_with_no_epoch_case_is_refused), - case!(only_the_owner_closes_a_stream), case!(an_append_whose_ids_would_not_fit_an_offset_is_refused_before_it_writes), - case!(every_store_keeps_the_meta_fields_of_the_shared_vectors), case!(a_producer_field_has_the_shared_layout), case!(a_producer_field_of_another_format_is_refused), ]; diff --git a/crates/streams/src/testing/support.rs b/crates/streams/src/testing/support.rs index 3e035a932..8cf34d010 100644 --- a/crates/streams/src/testing/support.rs +++ b/crates/streams/src/testing/support.rs @@ -7,14 +7,12 @@ use super::{RawKeys, backends}; use crate::{ StreamResult, StreamRuntime, digest::batch_digest, - epoch::withdraws, ids::local_activity_producer_id, proto::{ - AppendRecord, LocalActivityEpoch, LocalActivityProducer, Producer, RunWithdrawal, - StreamData, StreamRecord, StreamRef, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, - producer::Kind, stream_record::Item, + AppendRecord, LocalActivityEpoch, LocalActivityProducer, Producer, StreamData, + StreamRecord, StreamRef, WorkflowTaskEpoch, producer::Kind, stream_record::Item, }, - store::{Appended, Batch, Closed, Create, Created, Entry, Owner, Store, Withdrawal}, + store::{Appended, Batch, Closed, Create, Created, Entry, Store}, }; use prost::Message; use prost_types::Timestamp; @@ -22,7 +20,6 @@ use std::sync::{ Arc, atomic::{AtomicU64, Ordering}, }; -use temporalio_common::protos::temporal::api::{common::v1::Execution, enums::v1::ExecutionType}; static NEXT: AtomicU64 = AtomicU64::new(0); @@ -143,15 +140,6 @@ impl Run { } } -/// The owner of a ref that a Workflow's run chain makes, with the chain's first run id. -pub(crate) fn workflow_execution(workflow_id: &str, first_run_id: &str) -> Option { - Some(Execution { - r#type: ExecutionType::Workflow as i32, - business_id: workflow_id.to_string(), - run_id: first_run_id.to_string(), - }) -} - pub(crate) fn record(producer: &Producer, text: &str) -> Vec { StreamRecord { item: Some(Item::Data(StreamData { @@ -192,20 +180,6 @@ pub(crate) fn digest(texts: &[&str]) -> [u8; crate::digest::DIGEST_LEN] { batch_digest(&records) } -pub(crate) fn superseded(producer: &Producer, withdrawal: &Withdrawal<'_>) -> Vec { - let run = (!withdrawal.withdraw_run.is_empty()).then(|| RunWithdrawal { - withdraw_run: withdrawal.withdraw_run.to_string(), - ..Default::default() - }); - StreamSuperseded { - writer: Some(StreamWriter { - producer: Some(producer.clone()), - withdrawal: run, - }), - } - .encode_to_vec() -} - /// The data texts of the entries, in log order. pub(crate) fn records_of(entries: &[Entry]) -> Vec { entries @@ -227,13 +201,8 @@ impl Target { } } - pub(crate) async fn create(&self, owner: Owner) { - self.create_with(&Create { - owner, - ..Default::default() - }) - .await - .unwrap(); + pub(crate) async fn create(&self) { + self.create_with(&Create::default()).await.unwrap(); } /// Creates the Stream, and gives the Store's answer. @@ -289,69 +258,19 @@ impl Target { entries } - /// The data texts in the log, withdrawn or not. + /// The data texts in the log. pub(crate) async fn texts(&self) -> Vec { records_of(&self.entries().await) } - /// The data texts a Reader keeps after it applies each `Superseded` item. - pub(crate) async fn visible(&self) -> Vec { - let mut kept: Vec = vec![]; - for entry in self.entries().await { - if let Some(bytes) = &entry.superseded { - let s = StreamSuperseded::decode(bytes.as_slice()).unwrap(); - kept.retain(|old| !withdraws(&s, old)); - } - if let Some(bytes) = &entry.record { - let record = StreamRecord::decode(bytes.as_slice()).unwrap(); - if matches!(record.item, Some(Item::Data(_))) { - kept.push(record); - } - } - } - kept.into_iter() - .filter_map(|record| match record.item { - Some(Item::Data(data)) => Some(String::from_utf8(data.body.unwrap().data).unwrap()), - _ => None, - }) - .collect() - } - + /// Appends a batch of `texts` as `producer` with `sequence`. pub(crate) async fn batch( &self, producer: &Producer, sequence: i64, texts: &[&str], - ) -> StreamResult { - self.write(producer, sequence, "", texts).await - } - - /// A retry run's first batch, which withdraws `previous`. - pub(crate) async fn batch_retry( - &self, - producer: &Producer, - previous: &str, - texts: &[&str], - ) -> StreamResult { - self.write(producer, 1, previous, texts).await - } - - /// A batch that withdraws `withdraw_run`, or no run when it is empty. - pub(crate) async fn write( - &self, - producer: &Producer, - sequence: i64, - withdraw_run: &str, - texts: &[&str], ) -> StreamResult { let records: Vec<_> = texts.iter().map(|text| record(producer, text)).collect(); - let mut withdrawal = Withdrawal { - withdraw_run, - superseded: &[], - tasks: None, - }; - let bytes = superseded(producer, &withdrawal); - withdrawal.superseded = &bytes; self.store .append( &self.namespace, @@ -361,24 +280,14 @@ impl Target { sequence, records: &records, digest: digest(texts), - withdrawal, }, ) .await } pub(crate) async fn close(&self, producer: &Producer) -> StreamResult { - self.close_at(&self.stream_ref(), producer).await - } - - /// Closes the Stream of `stream_ref`, with the owner fields it has. - pub(crate) async fn close_at( - &self, - stream_ref: &StreamRef, - producer: &Producer, - ) -> StreamResult { self.store - .close(&self.namespace, stream_ref, producer) + .close(&self.namespace, &self.stream_ref(), producer) .await } } diff --git a/crates/streams/src/testing/writer.rs b/crates/streams/src/testing/writer.rs index c509f688d..2c411d76b 100644 --- a/crates/streams/src/testing/writer.rs +++ b/crates/streams/src/testing/writer.rs @@ -8,18 +8,13 @@ use super::{ }; use crate::{ MemoryStreamRuntime, StreamError, StreamResult, StreamRuntime, - epoch::withdraws, - ids::{ - activity_producer_id, client_producer_id, local_activity_producer_id, - named_client_producer_id, - }, + ids::{activity_producer_id, client_producer_id, named_client_producer_id}, proto::{ ActivityEpoch, ActivityProducer, AppendRecord, ClientEpoch, ClientProducer, CloseStreamRequest, CloseWriterRequest, CloseWriterResponse, CreateStreamRequest, - CreateStreamResponse, LocalActivityEpoch, LocalActivityProducer, OpenWriterRequest, - OpenWriterResponse, Producer, ReadRecordsRequest, ReadRecordsResponse, RunWithdrawal, - StreamErrorDetails, StreamRecord, StreamRef, StreamWriter, WorkflowTaskEpoch, - WriteRecordsRequest, WriteRecordsResponse, producer::Kind, stream_record::Item, + CreateStreamResponse, OpenWriterRequest, OpenWriterResponse, Producer, ReadRecordsRequest, + ReadRecordsResponse, StreamErrorDetails, StreamRef, StreamWriter, WriteRecordsRequest, + WriteRecordsResponse, producer::Kind, stream_record::Item, }, store::{ Appended, Batch, Capabilities, Closed, Create, Created, Entry, MAX_BATCH_RECORDS, Store, @@ -79,11 +74,10 @@ async fn cases() -> Vec { cases } -/// The writer of a write by `producer`, with no withdrawal facts. +/// The writer of a write by `producer`. fn writer_for(producer: Producer) -> Option { Some(StreamWriter { producer: Some(producer), - ..Default::default() }) } @@ -194,15 +188,10 @@ impl Case { } async fn visible(&self) -> Vec { - let mut kept: Vec = vec![]; - for record in self.read(0).await.records { - match &record.item { - Some(Item::Superseded(s)) => kept.retain(|old| !withdraws(s, old)), - Some(Item::Data(_)) => kept.push(record), - _ => {} - } - } - kept.into_iter() + self.read(0) + .await + .records + .into_iter() .filter_map(|record| match record.item { Some(Item::Data(data)) => Some(String::from_utf8(data.body.unwrap().data).unwrap()), _ => None, @@ -287,15 +276,6 @@ async fn a_failed_write_fails_every_later_write_and_the_close() { } } -async fn an_activity_retry_withdraws_the_earlier_attempt_even_with_nothing_to_write() { - for case in cases().await { - let first = case.open(activity(1), |_| {}).await; - case.write(first, &["a1"]).await.unwrap(); - case.open(activity(2), |_| {}).await; - assert!(case.visible().await.is_empty()); - } -} - /// An Activity retry opens its writer on a closed or expired Stream, so a retry that only reads /// the Stream still runs. Its first write fails with the real reason. async fn an_activity_retry_opens_on_a_closed_or_expired_stream() { @@ -340,6 +320,17 @@ async fn an_activity_retry_opens_on_a_closed_or_expired_stream() { }; let first = case.open(activity(1), on_short).await; case.write(first, &["a1"]).await.unwrap(); + let close_short = CloseStreamRequest { + namespace: case.namespace.clone(), + r#ref: Some(StreamRef { + id: short.clone(), + ..Default::default() + }), + writer: writer_for(client()), + ..Default::default() + }; + let _: crate::proto::CloseStreamResponse = + case.call("CloseStream", close_short).await.unwrap(); tokio::time::sleep(Duration::from_millis(500)).await; let retry = case.open(activity(2), on_short).await; let error = case.write(retry, &["a2"]).await.unwrap_err(); @@ -431,7 +422,7 @@ async fn a_named_client_with_a_higher_epoch_fences_its_earlier_self() { case.write(new, &["new"]).await.unwrap(); let error = case.write(old, &["late"]).await.unwrap_err(); assert_eq!(reason(&error), "stream_fenced"); - // The earlier epoch's Records are real messages, so the new epoch withdraws none. + // A fence blocks the old epoch's later writes but keeps its earlier Records. assert_eq!(case.visible().await, ["old", "new"]); } } @@ -642,37 +633,6 @@ async fn a_write_on_corrupt_store_data_fails_with_no_retry() { } } -/// An open of a writer with a run to withdraw fails with `stream_invalid` and writes nothing, since -/// only a retry run's first append withdraws a run and a writer sends many batches. -async fn a_writer_with_a_run_to_withdraw_is_refused() { - for case in cases().await { - let request = OpenWriterRequest { - namespace: case.namespace.clone(), - r#ref: Some(StreamRef { - id: case.stream.clone(), - ..Default::default() - }), - writer: Some(StreamWriter { - producer: Some(client()), - withdrawal: Some(RunWithdrawal { - withdraw_run: "r1".to_string(), - ..Default::default() - }), - }), - ..Default::default() - }; - let error = case - .call::<_, OpenWriterResponse>("OpenWriter", request) - .await - .unwrap_err(); - assert_eq!( - (error.code(), reason(&error).as_str()), - (Code::InvalidArgument, "stream_invalid") - ); - assert!(case.visible().await.is_empty()); - } -} - async fn a_write_that_waits_behind_a_close_is_refused() { for case in cases().await { let writer = case.open(client(), |_| {}).await; @@ -692,56 +652,12 @@ async fn a_write_that_waits_behind_a_close_is_refused() { } } -async fn a_local_activity_writer_withdraws_the_records_of_a_failed_task() { - for case in cases().await { - let local_activity = |call: &str, started: i64| Producer { - id: local_activity_producer_id("r1", call), - kind: Some(Kind::LocalActivity(LocalActivityProducer { - run_id: "r1".to_string(), - epoch: Some(LocalActivityEpoch { - task: Some(WorkflowTaskEpoch { - started_event_id: started, - attempt: 1, - started_time: Some(Timestamp { - seconds: started, - nanos: 0, - }), - }), - attempt: 1, - }), - })), - }; - let facts = |completed: i64, failed: Vec| { - move |request: &mut OpenWriterRequest| { - let writer = request.writer.get_or_insert_with(Default::default); - let withdrawal = writer.withdrawal.get_or_insert_with(Default::default); - withdrawal.previous_completed_task = Some(WorkflowTaskEpoch { - started_event_id: completed, - ..Default::default() - }); - withdrawal.failed_tasks = failed.clone(); - } - }; - // Task [3] fails after its Local Activity writes, and task [4] completes with no write. - let orphan = case.open(local_activity("1", 3), facts(2, vec![])).await; - case.write(orphan, &["orphan"]).await.unwrap(); - let next = case.open(local_activity("2", 5), facts(4, vec![3])).await; - case.write(next, &["next"]).await.unwrap(); - assert_eq!(case.visible().await, ["next"]); - // Task [5] completed while call 2 ran, and task [6], where the call resolved, failed. - // Call 2 runs again in task [8] and writes nothing. Its open withdraws its earlier task. - case.open(local_activity("2", 8), facts(5, vec![6])).await; - assert!(case.visible().await.is_empty()); - } -} - /// The cases of the suite, in file order. pub const CASES: &[super::Case] = &[ case!(a_write_is_in_the_store_when_it_answers_its_offsets), case!(writes_split_at_the_batch_size_and_keep_one_sequence), case!(a_batch_size_over_the_store_limit_writes_in_more_batches), case!(a_failed_write_fails_every_later_write_and_the_close), - case!(an_activity_retry_withdraws_the_earlier_attempt_even_with_nothing_to_write), case!(an_activity_retry_opens_on_a_closed_or_expired_stream), case!(a_closed_writer_is_gone), case!(a_write_splits_big_records_by_bytes_and_refuses_an_oversize_one), @@ -752,7 +668,5 @@ pub const CASES: &[super::Case] = &[ case!(a_write_whose_answer_is_lost_goes_in_once), case!(a_writer_writes_again_after_a_store_outage), case!(a_write_on_corrupt_store_data_fails_with_no_retry), - case!(a_writer_with_a_run_to_withdraw_is_refused), case!(a_write_that_waits_behind_a_close_is_refused), - case!(a_local_activity_writer_withdraws_the_records_of_a_failed_task), ]; diff --git a/crates/streams/src/writer.rs b/crates/streams/src/writer.rs index 88e31342f..e268c1474 100644 --- a/crates/streams/src/writer.rs +++ b/crates/streams/src/writer.rs @@ -8,7 +8,7 @@ use crate::{ Reason, StreamError, StreamResult, - epoch::{ProducerKind, writer_withdraw_run}, + epoch::ProducerKind, proto::{ AppendRecord, AppendRecordsRequest, OpenWriterRequest, Producer, StreamRef, StreamWriter, producer::Kind, @@ -218,9 +218,9 @@ pub(crate) struct Writers { impl Writers { /// Opens a writer, or adds a reference to the one that this Producer epoch has on the Stream. A - /// writer of a Local Activity, or of an Activity attempt above 1, first sends an empty batch. - /// So the new epoch withdraws the earlier one's Records even when it publishes nothing. On a - /// closed or expired Stream the open still succeeds, and the first write fails with the reason. + /// writer of a Local Activity, or of an Activity attempt above 1, first sends an empty batch, so + /// the new epoch sets its fence even when it publishes nothing. On a closed or expired Stream the + /// open still succeeds, and the first write fails with the reason. pub(crate) async fn open( &self, service: &Arc, @@ -231,10 +231,6 @@ impl Writers { .ok_or_else(|| StreamError::invalid("The writer has no Stream"))?; crate::ids::check_stream_id(&stream.id)?; let stream_writer = request.writer.unwrap_or_default(); - // Only a retry run's first append withdraws a run, and a writer sends many batches. - if !writer_withdraw_run(&stream_writer).is_empty() { - return Err(StreamError::invalid("A writer doesn't withdraw a run")); - } let producer = stream_writer .producer .clone() @@ -278,7 +274,7 @@ impl Writers { }); if retry { match writer.send(vec![], next_sequence()).await { - // A Stream with no key has nothing to withdraw yet. A closed or expired one accepts + // A Stream with no key has no fence to set yet. A closed or expired one accepts // no more Records, and a retry that only reads it must still run. Ok(_) => {} Err(error) diff --git a/crates/streams/testdata/fields.json b/crates/streams/testdata/fields.json index 036632040..6ed56fb8b 100644 --- a/crates/streams/testdata/fields.json +++ b/crates/streams/testdata/fields.json @@ -3,33 +3,24 @@ "producer_id": "act:run%3A1:a:s", "run_id": "run:1", "producer": "p|3|act:run%3A1:a:s", - "run_wrote": "run|run%3A1", - "withdrawn_run": "wr|run%3A1", "producer_values": [ { "name": "a batch with Records", "fence": "0000000000000000000000000000000000000000000000000000000068e778013b9ac9ff00000002", - "held": "0000000000000000000000000000000000000000000000000000000068e778013b9ac9ff00000002", "sequence": 3, "first": "1760000000123-4", "last": "1760000000123-6", "digest": "aca82bcf079b9b292bba1a09da05f527", - "value": "010000000000000000000000000000000000000000000000000000000068e778013b9ac9ff000000020000000000000000000000000000000000000000000000000000000068e778013b9ac9ff00000002000000000000000300000199c82cc07b0000000400000199c82cc07b00000006aca82bcf079b9b292bba1a09da05f527" + "value": "0000000000000000000000000000000000000000000000000000000068e778013b9ac9ff00000002000000000000000300000199c82cc07b0000000400000199c82cc07b00000006aca82bcf079b9b292bba1a09da05f527" }, { "name": "an empty batch that wrote no entry", "fence": "0000000000000000000000000000000000000000000000000000000068e778020000000000000003", - "held": null, "sequence": 1, "first": "", "last": "", "digest": "e3b0c44298fc1c149afbf4c8996fb924", - "value": "000000000000000000000000000000000000000000000000000000000068e778020000000000000003000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000e3b0c44298fc1c149afbf4c8996fb924" + "value": "0000000000000000000000000000000000000000000000000000000068e7780200000000000000030000000000000001000000000000000000000000000000000000000000000000e3b0c44298fc1c149afbf4c8996fb924" } - ], - "task_key": "000000000000000300000001000000000000006400000000", - "open_tasks": "oa|run%3A1", - "withdrawn_task_prefix": "ow|run%3A1|", - "task_facts": "tf|run%3A1", - "task_facts_value": "000000000000000500000001000000000000006400000000000000000000000400000000000000000000000000000000000000000000000300000000000000000000000000000000" + ] } diff --git a/crates/streams/testdata/gen_vectors.py b/crates/streams/testdata/gen_vectors.py index ab49e64f7..56e044514 100644 --- a/crates/streams/testdata/gen_vectors.py +++ b/crates/streams/testdata/gen_vectors.py @@ -140,28 +140,25 @@ def entry_id(text): return ms.to_bytes(8, "big") + seq.to_bytes(4, "big") -def producer_value(fence, held, sequence, first, last, digest): - """The 129 bytes of a `p|` field.""" - flags = bytes([1 if held else 0]) - return (flags + fence + (held or bytes(40)) + sequence.to_bytes(8, "big") + entry_id(first) - + entry_id(last) + digest) +def producer_value(fence, sequence, first, last, digest): + """The 88 bytes of a `p|` field.""" + return fence + sequence.to_bytes(8, "big") + entry_id(first) + entry_id(last) + digest def producer_values(): - held = epoch_key({"attempt_scheduled_time": [1760000001, 999999999], "attempt": 2}) - newer = epoch_key({"attempt_scheduled_time": [1760000002, 0], "attempt": 3}) + with_records = epoch_key({"attempt_scheduled_time": [1760000001, 999999999], "attempt": 2}) + empty = epoch_key({"attempt_scheduled_time": [1760000002, 0], "attempt": 3}) cases = [] - for name, fence, held_key, sequence, first, last, records in [ - ("a batch with Records", held, held, 3, "1760000000123-4", "1760000000123-6", + for name, fence, sequence, first, last, records in [ + ("a batch with Records", with_records, 3, "1760000000123-4", "1760000000123-6", [{"data": "01"}]), - ("an empty batch that wrote no entry", newer, None, 1, "", "", []), + ("an empty batch that wrote no entry", empty, 1, "", "", []), ]: batch_digest = bytes.fromhex(digest(records)) - value = producer_value(fence, held_key, sequence, first, last, batch_digest) + value = producer_value(fence, sequence, first, last, batch_digest) cases.append({ "name": name, "fence": fence.hex(), - "held": held_key.hex() if held_key else None, "sequence": sequence, "first": first, "last": last, @@ -174,23 +171,12 @@ def producer_values(): def fields(): run = "run:1" producer = "act:" + join(run, "a", "s") - # Task [3] fails, and a write of task [5] with the completed task [4] withdraws it. - withdrawn = task_key(3, 1, (100, 0)) - task, completed = task_key(5, 1, (100, 0)), task_key(4) return { "kind": 3, "producer_id": producer, "run_id": run, "producer": f"p|3|{producer}", - "run_wrote": f"run|{part(run)}", - "withdrawn_run": f"wr|{part(run)}", "producer_values": producer_values(), - # The task keys are binary, so the vectors give them in hex after the field name's prefix. - "task_key": withdrawn.hex(), - "open_tasks": f"oa|{part(run)}", - "withdrawn_task_prefix": f"ow|{part(run)}|", - "task_facts": f"tf|{part(run)}", - "task_facts_value": (task + completed + task_key(3)).hex(), } @@ -258,186 +244,12 @@ def digests(): return cases -def withdraws(superseded, record): - """The reader rule of a Superseded item, as the API proposal states it.""" - producer = record["producer"] - if superseded.get("withdraw_run") and producer.get("run_id") == superseded["withdraw_run"]: - return True - # A client's newer epoch only fences. Its earlier Records are real messages. - if superseded["producer_kind"] == "client": - return False - # An item with no epoch, such as a create's, withdraws only a run. - if "epoch" not in superseded or "epoch" not in producer: - return False - old, new = producer["epoch"], superseded["epoch"] - if superseded["producer_kind"] != "workflow": - return ( - producer["kind"] == superseded["producer_kind"] - and producer["id"] == superseded["producer_id"] - and epoch_key(old) < epoch_key(new) - ) - if producer["kind"] == "workflow": - if producer["id"] != superseded["producer_id"]: - return False - elif producer["kind"] == "local_activity": - if producer.get("run_id", "") != superseded["run_id"]: - return False - else: - return False - - def task(epoch): - return ( - epoch.get("task_started_event_id", 0), - task_key( - epoch.get("task_started_event_id", 0), - epoch.get("task_attempt", 0), - tuple(epoch.get("task_started_time", (0, 0))), - ), - ) - - started, old_key = task(old) - t_started, t_key = task(new) - c = superseded.get("previous_completed_task") - c_started = c["started_event_id"] if c else 0 - if started in superseded.get("failed_tasks", []): - return True - if c_started < started < t_started: - return True - if started == t_started and old_key < t_key: - return True - if c and started == c_started: - c_key = task_key(c_started, c.get("attempt", 0), tuple(c.get("started_time", (0, 0)))) - if old_key < c_key: - return True - return False - - -def wf(run, started, attempt=1, time=(100, 0)): - return {"kind": "workflow", "id": "wf:" + part(run), "run_id": run, - "epoch": {"task_started_event_id": started, "task_attempt": attempt, - "task_started_time": list(time)}} - - -def withdrawal_examples(): - """Appendix E of the API proposal, one case per Record that a Superseded item meets.""" - r1 = "r1" - cases = [] - - def case(name, superseded, producer, expected): - record = {"producer": producer} - assert withdraws(superseded, record) == expected, name - cases.append({"name": name, "superseded": superseded, "record": record, - "withdraws": expected}) - - def sup(task, c=None, failed=(), withdraw_run=None, run=r1): - s = {"producer_kind": "workflow", "producer_id": "wf:" + part(run), "run_id": run, - "epoch": task["epoch"], "failed_tasks": list(failed)} - if c: - s["previous_completed_task"] = c - if withdraw_run: - s["withdraw_run"] = withdraw_run - return s - - def completed(started, attempt=1, time=(100, 0)): - return {"started_event_id": started, "attempt": attempt, "started_time": list(time)} - - # 1. A retried task writes nothing. B at [5] failed, [8] wrote nothing, C at [11], c = [8]. - s = sup(wf(r1, 11), completed(8), failed=[5]) - case("e1 B at the failed task 5 goes", s, wf(r1, 5), True) - case("e1 a Record at the completed task 8 stays", s, wf(r1, 8), False) - case("e1 a Record before c stays", s, wf(r1, 3), False) - # 2. A refused completion. B at [5] refused, B' at [8] with failed [5]. - s = sup(wf(r1, 8), completed(2), failed=[5]) - case("e2 B goes", s, wf(r1, 5), True) - case("e2 an older completed Record stays", s, wf(r1, 2), False) - # 3. A transient retry. D at [8] attempt 2, D' at [8] attempt 3. - s = sup(wf(r1, 8, attempt=3), completed(5)) - case("e3 D at a lower attempt of t goes", s, wf(r1, 8, attempt=2), True) - case("e3 a Record of t itself stays", s, wf(r1, 8, attempt=3), False) - # 4. A dropped speculative task. U at [9] T1, the next task at [9] T2. - s = sup(wf(r1, 9, time=(200, 0)), completed(6)) - case("e4 U at an earlier started time goes", s, wf(r1, 9, time=(150, 0)), True) - # 5. A refused transient attempt. c = [5], D at [11] refused, D' at [14]. - s = sup(wf(r1, 14), completed(5)) - case("e5 D between c and t goes", s, wf(r1, 11, attempt=2), True) - case("e5 a Record at c stays", s, wf(r1, 5), False) - # c is the third attempt of task [8], as Core sends it. The transient attempts 1 and 2 have - # no failed event, so only c's attempt withdraws them. - s = sup(wf(r1, 14), completed(8, attempt=3, time=(300, 0))) - case("c attempt 3, attempt 2 at c goes", s, wf(r1, 8, attempt=2, time=(200, 0)), True) - case("c attempt 3, c itself stays", s, wf(r1, 8, attempt=3, time=(300, 0)), False) - # A c with only its started id, from a Core that sends no attempt, withdraws nothing at c. - s = sup(wf(r1, 14), {"started_event_id": 8}) - case("c with no attempt keeps every Record at c", s, wf(r1, 8, attempt=1, time=(200, 0)), - False) - # A Local Activity of a withdrawn task is withdrawn too. - la = {"kind": "local_activity", "id": "la:" + join(r1, "k"), "run_id": r1, - "epoch": {"task_started_event_id": 5, "task_attempt": 1, - "task_started_time": [100, 0], "attempt": 2}} - case("e1 a Local Activity of the failed task goes", sup(wf(r1, 11), completed(8), [5]), la, - True) - other_run = wf("r9", 5) - case("another run's Record stays", sup(wf(r1, 11), completed(8), [5]), other_run, False) - # 6. A Workflow retry. r2's first batch withdraws r1 and its Activity. - act = {"kind": "activity", "id": "act:" + join(r1, "a", "s"), "run_id": r1, - "epoch": {"attempt_scheduled_time": [100, 0], "attempt": 1}} - s = sup(wf("r2", 3), None, withdraw_run=r1, run="r2") - case("e6 r1's Record goes", s, wf(r1, 20), True) - case("e6 r1's Activity Record goes", s, act, True) - case("e6 r2's own earlier Record of t stays", s, wf("r2", 3), False) - # An Activity retry withdraws only its own earlier attempt. - s = {"producer_kind": "activity", "producer_id": act["id"], "run_id": r1, - "epoch": {"attempt_scheduled_time": [130, 0], "attempt": 2}} - case("an Activity retry withdraws attempt 1", s, act, True) - other = dict(act, id="act:" + join(r1, "b", "s")) - case("an Activity retry keeps another Activity", s, other, False) - # A client's newer epoch fences its earlier one and withdraws nothing. - named = {"kind": "client", "id": "client:" + join("ingest"), "epoch": {"attempt": 1}} - s = {"producer_kind": "client", "producer_id": named["id"], "epoch": {"attempt": 2}} - case("a client's newer epoch keeps its earlier Records", s, named, False) - - def la_at(started, task_attempt=1, attempt=1, run=r1, call="k"): - return {"kind": "local_activity", "id": "la:" + join(run, call), "run_id": run, - "epoch": {"task_started_event_id": started, "task_attempt": task_attempt, - "task_started_time": [100, 0], "attempt": attempt}} - - # A retry run's create writes an item with no epoch. It withdraws only the previous run, even - # when it names failed tasks. - s = {"producer_kind": "workflow", "producer_id": "wf:" + part("r2"), "run_id": "r2", - "withdraw_run": r1, "failed_tasks": [5]} - case("no epoch, r1's Record goes", s, wf(r1, 5), True) - case("no epoch, r1's Local Activity Record goes", s, la_at(5), True) - case("no epoch, r2's Local Activity Record at a failed task stays", s, la_at(5, run="r2"), - False) - case("no epoch, r2's Record stays", s, wf("r2", 5), False) - # A Local Activity's newer epoch withdraws only its own lower epochs. - s = {"producer_kind": "local_activity", "producer_id": "la:" + join(r1, "k"), "run_id": r1, - "epoch": la_at(8, attempt=3)["epoch"]} - case("a Local Activity's newer epoch withdraws a lower attempt", s, la_at(8, attempt=2), True) - case("a Local Activity's newer epoch withdraws an earlier task", s, la_at(5, attempt=4), True) - case("a Local Activity's newer epoch keeps its own epoch", s, la_at(8, attempt=3), False) - case("a Local Activity's newer epoch keeps another call", s, la_at(5, call="j"), False) - case("a Local Activity's item keeps the run's Workflow Record", s, wf(r1, 5), False) - # Local Activity Records under a Workflow item with c = [8], t = [14] attempt 2, failed [5]. - s = sup(wf(r1, 14, attempt=2), completed(8), failed=[5]) - case("a Local Activity before c stays", s, la_at(3), False) - case("a Local Activity at c stays", s, la_at(8), False) - case("a Local Activity between c and t goes", s, la_at(11), True) - case("a Local Activity of a lower attempt of t goes", s, la_at(14, task_attempt=1), True) - case("a Local Activity of t itself stays", s, la_at(14, task_attempt=2), False) - case("a Local Activity of another run stays", s, la_at(11, run="r9"), False) - no_epoch = {"kind": "local_activity", "id": "la:" + join(r1, "k"), "run_id": r1} - case("a Local Activity Record with no epoch stays", s, no_epoch, False) - return cases - - def main(): for name, value in [ ("ids.json", ids()), ("offsets.json", offsets()), ("epochs.json", epochs()), ("fields.json", fields()), - ("withdrawals.json", withdrawal_examples()), ("digests.json", digests()), ]: with open(name, "w") as out: diff --git a/crates/streams/testdata/withdrawals.json b/crates/streams/testdata/withdrawals.json deleted file mode 100644 index fdf395bf4..000000000 --- a/crates/streams/testdata/withdrawals.json +++ /dev/null @@ -1,1390 +0,0 @@ -[ - { - "name": "e1 B at the failed task 5 goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e1 a Record at the completed task 8 stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e1 a Record before c stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e2 B goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 2, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e2 an older completed Record stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 2, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 2, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e3 D at a lower attempt of t goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 3, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 5, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e3 a Record of t itself stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 3, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 5, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 3, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e4 U at an earlier started time goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 9, - "task_attempt": 1, - "task_started_time": [ - 200, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 6, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 9, - "task_attempt": 1, - "task_started_time": [ - 150, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e5 D between c and t goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 5, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e5 a Record at c stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 5, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "c attempt 3, attempt 2 at c goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 3, - "started_time": [ - 300, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 2, - "task_started_time": [ - 200, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "c attempt 3, c itself stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 3, - "started_time": [ - 300, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 3, - "task_started_time": [ - 300, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "c with no attempt keeps every Record at c", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "previous_completed_task": { - "started_event_id": 8 - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 200, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e1 a Local Activity of the failed task goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 2 - } - } - }, - "withdraws": true - }, - { - "name": "another run's Record stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r9", - "run_id": "r9", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "e6 r1's Record goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "withdraw_run": "r1" - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 20, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "e6 r1's Activity Record goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "withdraw_run": "r1" - }, - "record": { - "producer": { - "kind": "activity", - "id": "act:r1:a:s", - "run_id": "r1", - "epoch": { - "attempt_scheduled_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": true - }, - { - "name": "e6 r2's own earlier Record of t stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [], - "withdraw_run": "r1" - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r2", - "run_id": "r2", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "an Activity retry withdraws attempt 1", - "superseded": { - "producer_kind": "activity", - "producer_id": "act:r1:a:s", - "run_id": "r1", - "epoch": { - "attempt_scheduled_time": [ - 130, - 0 - ], - "attempt": 2 - } - }, - "record": { - "producer": { - "kind": "activity", - "id": "act:r1:a:s", - "run_id": "r1", - "epoch": { - "attempt_scheduled_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": true - }, - { - "name": "an Activity retry keeps another Activity", - "superseded": { - "producer_kind": "activity", - "producer_id": "act:r1:a:s", - "run_id": "r1", - "epoch": { - "attempt_scheduled_time": [ - 130, - 0 - ], - "attempt": 2 - } - }, - "record": { - "producer": { - "kind": "activity", - "id": "act:r1:b:s", - "run_id": "r1", - "epoch": { - "attempt_scheduled_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a client's newer epoch keeps its earlier Records", - "superseded": { - "producer_kind": "client", - "producer_id": "client:ingest", - "epoch": { - "attempt": 2 - } - }, - "record": { - "producer": { - "kind": "client", - "id": "client:ingest", - "epoch": { - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "no epoch, r1's Record goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "withdraw_run": "r1", - "failed_tasks": [ - 5 - ] - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": true - }, - { - "name": "no epoch, r1's Local Activity Record goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "withdraw_run": "r1", - "failed_tasks": [ - 5 - ] - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": true - }, - { - "name": "no epoch, r2's Local Activity Record at a failed task stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "withdraw_run": "r1", - "failed_tasks": [ - 5 - ] - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r2:k", - "run_id": "r2", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "no epoch, r2's Record stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r2", - "run_id": "r2", - "withdraw_run": "r1", - "failed_tasks": [ - 5 - ] - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r2", - "run_id": "r2", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity's newer epoch withdraws a lower attempt", - "superseded": { - "producer_kind": "local_activity", - "producer_id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 2 - } - } - }, - "withdraws": true - }, - { - "name": "a Local Activity's newer epoch withdraws an earlier task", - "superseded": { - "producer_kind": "local_activity", - "producer_id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 4 - } - } - }, - "withdraws": true - }, - { - "name": "a Local Activity's newer epoch keeps its own epoch", - "superseded": { - "producer_kind": "local_activity", - "producer_id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity's newer epoch keeps another call", - "superseded": { - "producer_kind": "local_activity", - "producer_id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:j", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity's item keeps the run's Workflow Record", - "superseded": { - "producer_kind": "local_activity", - "producer_id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 3 - } - }, - "record": { - "producer": { - "kind": "workflow", - "id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 5, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ] - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity before c stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 3, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity at c stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 8, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity between c and t goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": true - }, - { - "name": "a Local Activity of a lower attempt of t goes", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": true - }, - { - "name": "a Local Activity of t itself stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity of another run stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r9:k", - "run_id": "r9", - "epoch": { - "task_started_event_id": 11, - "task_attempt": 1, - "task_started_time": [ - 100, - 0 - ], - "attempt": 1 - } - } - }, - "withdraws": false - }, - { - "name": "a Local Activity Record with no epoch stays", - "superseded": { - "producer_kind": "workflow", - "producer_id": "wf:r1", - "run_id": "r1", - "epoch": { - "task_started_event_id": 14, - "task_attempt": 2, - "task_started_time": [ - 100, - 0 - ] - }, - "failed_tasks": [ - 5 - ], - "previous_completed_task": { - "started_event_id": 8, - "attempt": 1, - "started_time": [ - 100, - 0 - ] - } - }, - "record": { - "producer": { - "kind": "local_activity", - "id": "la:r1:k", - "run_id": "r1" - } - }, - "withdraws": false - } -] diff --git a/crates/streams/tests/capabilities.rs b/crates/streams/tests/capabilities.rs index 34212ccc9..f5719f2c9 100644 --- a/crates/streams/tests/capabilities.rs +++ b/crates/streams/tests/capabilities.rs @@ -6,7 +6,6 @@ use std::{ sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, }; -use temporalio_common::protos::temporal::api::{common::v1::Execution, enums::v1::ExecutionType}; use temporalio_streams::{ MemoryStreamRuntime, StreamError, StreamResult, StreamRuntime, ids::client_producer_id, @@ -15,7 +14,7 @@ use temporalio_streams::{ AppendRecord, AppendRecordsRequest, ClientEpoch, ClientProducer, CloseStreamRequest, CreateStreamRequest, DescribeStreamRequest, DescribeStreamResponse, Producer, StreamErrorDetails, StreamErrorReason, StreamRef, StreamStatus, StreamWriter, - WorkflowProducer, WorkflowTaskEpoch, producer::Kind, + producer::Kind, }, store::{Appended, Batch, Capabilities, Closed, Create, Created, Entry, Store, StreamState}, }; @@ -91,7 +90,6 @@ const NAMESPACE: &str = "ns"; struct Case { ext: Arc, memory: Arc, - run_id: String, stream: String, } @@ -108,12 +106,10 @@ fn case(without: Capabilities) -> Case { .duration_since(UNIX_EPOCH) .unwrap() .as_nanos(); - let run_id = format!("run-{unique}"); Case { ext: MemoryStreamRuntime::with_store(store, None), memory, - stream: format!("wf:{run_id}"), - run_id, + stream: format!("wf:run-{unique}"), } } @@ -126,11 +122,10 @@ fn client() -> Producer { } } -/// The writer of a client's write, with no withdrawal facts. +/// The writer of a client's write. fn client_writer() -> Option { Some(StreamWriter { producer: Some(client()), - ..Default::default() }) } @@ -171,60 +166,23 @@ impl Case { fn log_len(&self) -> usize { self.memory.test_log_len(NAMESPACE, &self.stream) } - - fn created(&self) -> bool { - self.memory - .test_meta(NAMESPACE, &self.stream, b"v") - .is_some() - } - - /// A create from a Workflow's call outside Workflow code, which names the owning run. - async fn create_owned(&self, withdraw_run: &str) -> Result, StreamError> { - let request = CreateStreamRequest { - namespace: NAMESPACE.to_string(), - withdraw_run: withdraw_run.to_string(), - owner: Some(Producer { - id: temporalio_streams::ids::workflow_producer_id(&self.run_id), - kind: Some(Kind::Workflow(WorkflowProducer { - run_id: self.run_id.clone(), - epoch: Some(WorkflowTaskEpoch::default()), - })), - }), - stream_ref: Some(StreamRef { - id: self.stream.clone(), - owner: Some(Execution { - r#type: ExecutionType::Workflow as i32, - business_id: "agent".to_string(), - run_id: self.run_id.clone(), - }), - ..Default::default() - }), - ..Default::default() - }; - self.call("CreateStream", request).await - } } #[tokio::test] -async fn an_append_without_the_fence_or_the_flag_is_refused_with_nothing_written() { - for (capability, name) in [ - (Capabilities::PRODUCER_FENCE, "producer_fence"), - (Capabilities::WITHDRAWAL, "withdrawal"), - ] { - let case = case(capability); - case.create().await; - let request = AppendRecordsRequest { - namespace: NAMESPACE.to_string(), - r#ref: case.stream_ref(), - writer: client_writer(), - sequence: 1, - records: vec![AppendRecord::default()], - ..Default::default() - }; - let error = case.call("AppendRecords", request).await.unwrap_err(); - assert_eq!(refused_capability(&error), name); - assert_eq!(case.log_len(), 0); - } +async fn an_append_without_the_fence_is_refused_with_nothing_written() { + let case = case(Capabilities::PRODUCER_FENCE); + case.create().await; + let request = AppendRecordsRequest { + namespace: NAMESPACE.to_string(), + r#ref: case.stream_ref(), + writer: client_writer(), + sequence: 1, + records: vec![AppendRecord::default()], + ..Default::default() + }; + let error = case.call("AppendRecords", request).await.unwrap_err(); + assert_eq!(refused_capability(&error), "producer_fence"); + assert_eq!(case.log_len(), 0); } #[tokio::test] @@ -274,22 +232,3 @@ async fn a_close_without_the_capability_is_refused_and_the_stream_stays_open() { let described = DescribeStreamResponse::decode(bytes.as_slice()).unwrap(); assert_eq!(described.status, StreamStatus::Open as i32); } - -#[tokio::test] -async fn a_workflow_create_without_the_safety_ttl_is_refused_before_it_writes() { - let case = case(Capabilities::SAFETY_TTL); - let error = case.create_owned("").await.unwrap_err(); - assert_eq!(refused_capability(&error), "safety_ttl"); - assert!(!case.created()); -} - -#[tokio::test] -async fn a_retry_run_without_the_run_fence_is_refused_before_it_writes() { - let case = case(Capabilities::RUN_FENCE); - let error = case.create_owned("previous-run").await.unwrap_err(); - assert_eq!(refused_capability(&error), "run_fence"); - assert!(!case.created()); - // A run that isn't a retry needs no run fence. - case.create_owned("").await.unwrap(); - assert!(case.created()); -} diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index e2e5f54c4..b6b7617bd 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -169,7 +169,6 @@ async fn append_sized( r#ref: stream_ref(), writer: Some(StreamWriter { producer: Some(client()), - ..Default::default() }), sequence, records, diff --git a/crates/streams/tests/vectors.rs b/crates/streams/tests/vectors.rs index d824561a4..fcf296e3b 100644 --- a/crates/streams/tests/vectors.rs +++ b/crates/streams/tests/vectors.rs @@ -8,14 +8,13 @@ use temporalio_common::protos::temporal::api::common::v1::{ }; use temporalio_streams::{ digest::batch_digest, - epoch::{epoch_key, task_key, task_of, withdraws}, + epoch::{epoch_key, task_key, task_of}, ids::{self, StreamKeys}, offset::{entry_id_of_offset, offset_of_entry_id}, proto::{ ActivityEpoch, ActivityProducer, AppendRecord, ClientEpoch, ClientProducer, - LocalActivityEpoch, LocalActivityProducer, Producer, RunWithdrawal, StreamData, - StreamRecord, StreamSuperseded, StreamWriter, WorkflowProducer, WorkflowTaskEpoch, - producer::Kind, stream_record::Item, + LocalActivityEpoch, LocalActivityProducer, WorkflowProducer, WorkflowTaskEpoch, + producer::Kind, }, }; @@ -159,68 +158,6 @@ fn epoch_keys_match() { } } -#[test] -fn the_appendix_e_withdrawals_match() { - let cases = load("withdrawals.json"); - assert!(cases.as_array().unwrap().len() >= 35); - for case in cases.as_array().unwrap() { - let s = &case["superseded"]; - // A create's item has no epoch, and a Reader must not read it as an empty one. - let withdrawing = Producer { - id: text(s, "producer_id").to_string(), - kind: s.get("epoch").and_then(|value| { - kind_of( - text(s, "producer_kind"), - value, - s["run_id"].as_str().unwrap_or_default(), - ) - }), - }; - let superseded = StreamSuperseded { - writer: Some(StreamWriter { - producer: Some(withdrawing), - withdrawal: Some(RunWithdrawal { - previous_completed_task: s.get("previous_completed_task").map(|c| { - WorkflowTaskEpoch { - started_event_id: number(c, "started_event_id"), - attempt: number(c, "attempt") as i32, - started_time: time(&c["started_time"]), - } - }), - failed_tasks: s["failed_tasks"] - .as_array() - .map(|list| list.iter().map(|v| v.as_i64().unwrap()).collect()) - .unwrap_or_default(), - withdraw_run: s["withdraw_run"].as_str().unwrap_or_default().to_string(), - }), - }), - }; - let p = &case["record"]["producer"]; - let record = StreamRecord { - item: Some(Item::Data(StreamData { - producer: Some(Producer { - id: text(p, "id").to_string(), - kind: p.get("epoch").and_then(|value| { - kind_of( - text(p, "kind"), - value, - p["run_id"].as_str().unwrap_or_default(), - ) - }), - }), - ..Default::default() - })), - ..Default::default() - }; - assert_eq!( - withdraws(&superseded, &record), - case["withdraws"].as_bool().unwrap(), - "{}", - text(case, "name") - ); - } -} - fn hex(text: &str) -> Vec { (0..text.len()) .step_by(2)