From 84fb6ca9dd9125cdddd7b51debd927604463868e Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Mon, 26 Oct 2026 22:21:23 -0700 Subject: [PATCH 01/11] Served reads with long polls, last-N reads and byte caps. A read waits for the create and for new Records, and fails on a gap that the Retention left. The service suite that reads back what the writes stored comes here too. --- crates/streams/src/lib.rs | 1 + crates/streams/src/metrics.rs | 13 +- crates/streams/src/runtime.rs | 9 +- crates/streams/src/service.rs | 370 +++++++++++- crates/streams/src/testing/service.rs | 811 +++++++++++++++++++++++++- crates/streams/tests/scans.rs | 235 ++++++++ 6 files changed, 1427 insertions(+), 12 deletions(-) create mode 100644 crates/streams/tests/scans.rs diff --git a/crates/streams/src/lib.rs b/crates/streams/src/lib.rs index add6490ac..2667f683b 100644 --- a/crates/streams/src/lib.rs +++ b/crates/streams/src/lib.rs @@ -23,6 +23,7 @@ pub mod testing; pub use error::{Reason, StreamError, StreamResult}; pub use runtime::{MEMORY_RUNTIME_NAME, MemoryStreamRuntime, StoreFactory, StreamRuntime}; +pub use service::{MAX_RECORDS, MAX_WAIT}; /// The protos that lang and the crate exchange. pub mod proto { diff --git a/crates/streams/src/metrics.rs b/crates/streams/src/metrics.rs index b894762e7..42824b437 100644 --- a/crates/streams/src/metrics.rs +++ b/crates/streams/src/metrics.rs @@ -1,7 +1,7 @@ //! The crate's metrics. They use the meter that lang gives, or a no-op meter. use temporalio_common::telemetry::metrics::{ - Counter, MetricAttributes, MetricParameters, TemporalMeter, + Counter, MetricAttributes, MetricParameters, TemporalMeter, UpDownCounter, }; #[derive(Clone)] @@ -9,6 +9,7 @@ pub(crate) struct Metrics { attributes: MetricAttributes, fenced: Counter, superseded: Counter, + blocking_reads_waiting: UpDownCounter, } fn parameters( @@ -37,6 +38,11 @@ impl Metrics { "Superseded flags that the Store set", "", )), + blocking_reads_waiting: meter.up_down_counter(parameters( + "stream_blocking_reads_waiting", + "Reads that wait on the Store", + "", + )), } } @@ -47,4 +53,9 @@ impl Metrics { 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/runtime.rs b/crates/streams/src/runtime.rs index 6c2143b19..8b76d9c06 100644 --- a/crates/streams/src/runtime.rs +++ b/crates/streams/src/runtime.rs @@ -4,7 +4,10 @@ use crate::{ Reason, StreamError, StreamResult, memory::MemoryStore, metrics::Metrics, - proto::{AppendRecordsRequest, CloseStreamRequest, DescribeStreamRequest, MemoryStreamConfig}, + proto::{ + AppendRecordsRequest, CloseStreamRequest, DescribeStreamRequest, MemoryStreamConfig, + ReadRecordsRequest, + }, service::StreamService, store::{DEFAULT_RETENTION, DEFAULT_SAFETY_TTL, Store}, }; @@ -132,6 +135,10 @@ impl StreamRuntime { .close_stream(decode::(input)?) .await? .encode_to_vec(), + "ReadRecords" => service + .read_records(decode::(input)?) + .await? + .encode_to_vec(), "DescribeStream" => service .describe_stream(decode::(input)?) .await? diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index 055790160..44856eb4d 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -1,23 +1,38 @@ -//! The Stream service operations on the Store, for Activities, clients and the Local Activity that -//! runs each Workflow call. +//! The five Stream service operations on the Store, for Activities, clients and the Local +//! Activity that runs each Workflow call. use crate::{ Reason, StreamError, StreamResult, digest::batch_digest, + epoch::withdraws, ids::{check_producer, check_stream_id}, metrics::Metrics, proto::{ AppendRecordsRequest, AppendRecordsResponse, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, - Producer, StreamProducerKind, StreamRef, StreamStatus, + Producer, ReadRecordsRequest, ReadRecordsResponse, StreamProducerKind, StreamReadStart, + StreamRecord, StreamRef, StreamStatus, Supersession, stream_record::Item, }, record::{caller_status, close_record, data_records, supersession}, store::{ - Batch, Capabilities, CreateOutcome, MAX_RECORD_BYTES, MAX_SEQUENCE, MAX_TTL, Owner, Store, - Withdrawal, owner_chain, + Batch, Capabilities, CreateOutcome, FORMAT_VERSION, MAX_RECORD_BYTES, MAX_SEQUENCE, + MAX_TTL, Owner, Store, StreamState, Withdrawal, owner_chain, }, }; +use prost::Message; use std::{sync::Arc, time::Duration}; +use tokio::time::Instant; + +/// The longest wait of one read, for new Records and for the create. +pub const MAX_WAIT: Duration = Duration::from_secs(60); +/// The maximum number of Records in one read. +pub const MAX_RECORDS: usize = 1000; +/// The maximum number of log entries that a read of the last Records scans. +const MAX_SCAN: usize = 10 * MAX_RECORDS; +/// The most entry bytes that a read of the last Records scans. +const MAX_SCAN_BYTES: usize = 64 * 1024 * 1024; +/// How often a read looks again for a Stream that nobody created yet. +const CREATE_POLL: Duration = Duration::from_millis(100); /// The Stream service on one Store. pub(crate) struct StreamService { @@ -305,6 +320,351 @@ impl StreamService { retention: Some(proto_duration(state.retention)), }) } + + /// A long poll that returns the Records it found, the close Record, or nothing after `wait`. + pub(crate) async fn read_records( + &self, + request: ReadRecordsRequest, + ) -> StreamResult { + let id = stream_id(request.stream.as_ref())?.to_string(); + let namespace = request.namespace.as_str(); + if request.from_offset < 0 { + return Err(StreamError::invalid("The offset is below zero")); + } + let max_bytes = max_bytes(request.max_bytes)?; + if request.last < 0 { + return Err(StreamError::invalid("last must be 0 or more")); + } + if request.last > 0 { + if request.wake_only || request.from_offset > 0 { + return Err(StreamError::invalid( + "A read of the last Records takes no from_offset and no wake_only", + )); + } + return self.read_last(request, max_bytes).await; + } + if request.wake_only || request.to_offset != 0 { + return Err(StreamError::new( + Reason::Unsupported, + "The Store reads no to_offset or wake_only, which the native backend serves", + )); + } + let max = match request.max_records { + n if n <= 0 => MAX_RECORDS, + n => (n as usize).min(MAX_RECORDS), + }; + let started = Instant::now(); + let wait = duration(request.wait).unwrap_or_default().min(MAX_WAIT); + let create_wait = duration(request.create_wait) + .unwrap_or_default() + .min(MAX_WAIT); + let deadline = started + wait; + + let mut state = self.created(namespace, &id, started + create_wait).await?; + if state.version > FORMAT_VERSION { + return Err(StreamError::new( + Reason::FormatTooNew, + format!("format {} is newer than {FORMAT_VERSION}", state.version), + )); + } + let from = match request.from_offset { + 0 if request.start == StreamReadStart::End as i32 => state.newest + 1, + from => from, + }; + if from > state.newest + 1 { + let next = state.newest + 1; + return Err(StreamError::new( + Reason::Offset, + format!( + "The offset {from} is past the newest offset + 1, {next}. A failover may have \ + lost the newest Records. Read from {next}, or read again once newer Records \ + pass the offset" + ), + )); + } + loop { + check_expiry(&state, from)?; + if let Some(page) = self + .page(namespace, &id, &state, from, (max, max_bytes)) + .await? + { + return Ok(page); + } + if deadline <= Instant::now() { + return Ok(ReadRecordsResponse { + next_offset: from, + ..Default::default() + }); + } + let after = (from - 1).max(state.newest); + self.metrics.read_waits(true); + let woke = self.store.wait_after(namespace, &id, after, deadline).await; + self.metrics.read_waits(false); + woke?; + state = self.store.read(namespace, &id, 0, 0).await?.0; + } + } + + /// The newest `last` Records, oldest first, and the close Record of a closed Stream at the end. + /// 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. + async fn read_last( + &self, + request: ReadRecordsRequest, + max_bytes: usize, + ) -> StreamResult { + let id = stream_id(request.stream.as_ref())?.to_string(); + let namespace = request.namespace.as_str(); + let count = (request.last as usize).min(MAX_RECORDS); + let create_wait = duration(request.create_wait) + .unwrap_or_default() + .min(MAX_WAIT); + let state = self + .created(namespace, &id, Instant::now() + create_wait) + .await?; + if state.version > FORMAT_VERSION { + return Err(StreamError::new( + Reason::FormatTooNew, + format!("format {} is newer than {FORMAT_VERSION}", state.version), + )); + } + check_expiry(&state, 0)?; + let retained = state.retained_from(); + let (mut picked, mut close) = (vec![], None); + 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 + && picked.len() < count + && scanned < MAX_SCAN + && scanned_bytes < MAX_SCAN_BYTES + { + let left = (count - picked.len()).clamp(16, MAX_RECORDS); + let room = max_bytes + .saturating_sub(bytes) + .min(MAX_SCAN_BYTES.saturating_sub(scanned_bytes)); + let chunk = chunk_len(left, Some(room), (scanned_bytes, scanned)); + let entries = self.store.read_back(namespace, &id, before, chunk).await?; + if entries.is_empty() { + break; + } + for entry in entries { + if entry.offset < retained { + break 'scan; + } + before = entry.offset - 1; + scanned += 1; + scanned_bytes += entry.record.as_ref().map_or(0, Vec::len) + + entry.supersession.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)?; + let size = record.encoded_len(); + // The close Record ends the page and doesn't count toward the byte cap. + if matches!(record.item, Some(Item::Close(_))) { + close = Some(record); + } else 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; + } + } + } + if let Some(supersession) = entry.supersession { + withdrawals.push( + Supersession::decode(supersession.as_slice()) + .map_err(|error| StreamError::storage(error.to_string()))?, + ); + } + } + } + picked.reverse(); + picked.extend(close); + Ok(ReadRecordsResponse { + records: picked, + next_offset: state.newest + 1, + done: state.closed_at.is_some(), + ..Default::default() + }) + } + + /// The state of a Stream, after waiting up to `until` for its create. + async fn created( + &self, + namespace: &str, + id: &str, + until: Instant, + ) -> StreamResult { + loop { + let (state, _) = self.store.read(namespace, id, 0, 0).await?; + if state.exists { + return Ok(state); + } + let now = Instant::now(); + if now >= until { + return Err(StreamError::new( + Reason::NotFound, + "Nobody created the Stream", + )); + } + tokio::time::sleep(CREATE_POLL.min(until - now)).await; + } + } + + /// The Records from `from`, or `None` when there is none yet. A page that ends in the close + /// Record is done. `(max, max_bytes)` caps the items and their bytes. + async fn page( + &self, + namespace: &str, + id: &str, + state: &StreamState, + from: i64, + (max, max_bytes): (usize, usize), + ) -> StreamResult> { + // A read past the close returns the close Record again, so that every read ends with it. + if let Some(closed_at) = state.closed_at.filter(|at| from > *at) { + let (_, entries) = self.store.read(namespace, id, closed_at, 1).await?; + let mut records = vec![]; + for entry in entries { + if let Some(bytes) = entry.record { + records.push(decode(&bytes, entry.offset)?); + } + } + let page = ReadRecordsResponse { + records, + next_offset: from, + done: true, + ..Default::default() + }; + return Ok(Some(page)); + } + // A read from the start skips the Records that the Retention hides. A read from an offset + // fails on them, since the Reader asked for those Records. + let retained = state.retained_from(); + let mut start = if from > 0 { from } else { retained }; + let mut records = vec![]; + let mut next = from; + let (mut done, mut full) = (false, false); + let mut bytes = 0; + // The bytes and the entries that the page read from the Store. + let mut pulled = (0, 0); + while records.len() < max && !done && !full { + let room = (max_bytes != usize::MAX).then(|| max_bytes.saturating_sub(bytes)); + let chunk = chunk_len(max - records.len(), room, pulled); + let (chunk_state, entries) = self.store.read(namespace, id, start, chunk).await?; + // A trim after the state read leaves a gap before these entries. + check_expiry(&chunk_state, if from > 0 { start } else { 0 })?; + 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 { + return Err(expired()); + } + break; + } + for entry in entries { + if entry.offset < retained { + if from > 0 { + return Err(expired()); + } + next = entry.offset + 1; + start = next; + continue; + } + let mut items = vec![]; + if let Some(bytes) = entry.supersession { + let supersession = Supersession::decode(bytes.as_slice()) + .map_err(|error| StreamError::storage(error.to_string()))?; + items.push(StreamRecord { + offset: entry.offset, + item: Some(Item::Superseded(supersession)), + }); + } + if let Some(bytes) = entry.record { + let record = decode(&bytes, entry.offset)?; + done = matches!(record.item, Some(Item::Close(_))); + items.push(record); + } + // 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(); + pulled = (pulled.0 + size, pulled.1 + 1); + let over = bytes + size > max_bytes || records.len() + items.len() > max; + if !records.is_empty() && over { + (done, full) = (false, true); + next = entry.offset; + break; + } + bytes += size; + records.extend(items); + next = entry.offset + 1; + start = next; + if done || records.len() >= max { + break; + } + } + } + if records.is_empty() { + return Ok(None); + } + let page = ReadRecordsResponse { + records, + next_offset: next, + done, + ..Default::default() + }; + Ok(Some(page)) + } +} + +/// The number of entries of the next Store read, at most `left`. With a byte cap, the first read +/// takes one entry, and each later one takes what fits the `room` left at the average size so far. +fn chunk_len(left: usize, room: Option, (bytes, entries): (usize, usize)) -> usize { + let Some(room) = room else { + return left; + }; + if entries == 0 { + return 1; + } + let average = (bytes / entries).max(1); + (room / average + 1).clamp(1, left.max(1)) +} + +fn decode(bytes: &[u8], offset: i64) -> StreamResult { + let mut record = StreamRecord::decode(bytes) + .map_err(|error| StreamError::storage(format!("A stored Record is bad: {error}")))?; + record.offset = offset; + Ok(record) +} + +/// Fails a read whose Records the Retention or the expiry removed, so that the Reader always sees +/// the gap. A read from the start of the Stream has no gap. +/// +/// A Tombstone fails every read. An open Stream that expired gets no more Records, so the Reader +/// would wait forever. A closed one lost its log and its close Record. +fn check_expiry(state: &StreamState, from: i64) -> StreamResult<()> { + if state.expired() || (from > 0 && state.trimmed >= from) { + return Err(expired()); + } + Ok(()) +} + +/// The byte cap of a page, from a request's `max_bytes`. Zero is no cap. +fn max_bytes(value: i64) -> StreamResult { + match value { + 0 => Ok(usize::MAX), + n if n < 0 => Err(StreamError::invalid("max_bytes must be 0 or more")), + n => Ok(usize::try_from(n).unwrap_or(usize::MAX)), + } +} + +fn expired() -> StreamError { + StreamError::new(Reason::Expired, "The Records at the offset expired") } /// A Store answer that may have withdrawn Records. diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 8808b9d44..1df4a0cee 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -6,22 +6,31 @@ use super::{case, support::Needs}; use crate::{ StreamError, StreamRuntime, - ids::client_producer_id, + epoch::withdraws, + ids::{activity_producer_id, client_producer_id, named_client_producer_id}, proto::{ AppendRecord, AppendRecordsRequest, AppendRecordsResponse, CloseStreamRequest, - CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, - StreamErrorDetails, StreamProducerKind, StreamRef, + CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, + DescribeStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, + StreamClose, StreamCloseStatus, StreamErrorDetails, StreamProducerKind, StreamReadStart, + StreamRecord, StreamRef, StreamStatus, Supersession, stream_record::Item, }, }; use prost::Message; -use std::{sync::Arc, time::Duration}; -use temporalio_common::protos::temporal::api::common::v1::Payload; +use prost_types::Timestamp; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; +use temporalio_common::protos::temporal::api::{common::v1::Payload, failure::v1::Failure}; use tonic::Code; 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); @@ -46,6 +55,7 @@ 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,6 +88,37 @@ fn client(attempt: i32) -> Producer { } } +/// A client Producer that a test names, at `attempt`. Every backend accepts it as sent. +fn named(name: &str, attempt: i32) -> Producer { + Producer { + kind: StreamProducerKind::Client as i32, + id: named_client_producer_id(name), + epoch: Some(ProducerEpoch { + attempt, + ..Default::default() + }), + run_id: String::new(), + } +} + +/// 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 { + kind: StreamProducerKind::Activity as i32, + id: activity_producer_id("r1", "a", "s"), + epoch: Some(ProducerEpoch { + attempt_scheduled_time: Some(Timestamp { + seconds: scheduled, + nanos: 0, + }), + attempt, + ..Default::default() + }), + run_id: "r1".to_string(), + } +} + fn record(text: &str) -> AppendRecord { AppendRecord { body: Some(Payload { @@ -140,6 +181,76 @@ impl Case { ) .await } + + async fn read(&self, request: ReadRecordsRequest) -> Result { + call( + &self.ext, + "ReadRecords", + ReadRecordsRequest { + namespace: self.namespace.clone(), + stream: self.stream(), + ..request + }, + ) + .await + } + + async fn describe(&self) -> Result { + call( + &self.ext, + "DescribeStream", + DescribeStreamRequest { + namespace: self.namespace.clone(), + stream: self.stream(), + }, + ) + .await + } + + async fn close(&self, producer: Producer) -> Result { + call( + &self.ext, + "CloseStream", + CloseStreamRequest { + namespace: self.namespace.clone(), + stream: self.stream(), + producer: Some(producer), + result: Some(Payload { + data: b"done".to_vec(), + ..Default::default() + }), + ..Default::default() + }, + ) + .await + } +} + +fn texts(response: &ReadRecordsResponse) -> Vec { + response + .records + .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) -> &Supersession { + match &record.item { + Some(Item::Superseded(supersession)) => supersession, + other => panic!("not a Superseded item: {other:?}"), + } +} + +fn close_of(record: &StreamRecord) -> &StreamClose { + match &record.item { + Some(Item::Close(close)) => close, + other => panic!("not the close: {other:?}"), + } } /// A create of a Stream that exists returns it with `created` false, whatever its Retention. A @@ -299,6 +410,231 @@ async fn a_producer_must_match_its_kind_and_not_be_a_workflow() { } } +/// A read is inclusive from its offset, a read from the end gives only new Records, an offset past +/// the newest + 1 fails with `stream_offset`, and one below zero is invalid. +async fn a_read_is_inclusive_from_its_offset_and_get_offset_reads_only_new_records() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let first = case + .append(client(1), 1, vec![record("1"), record("2")]) + .await + .unwrap(); + let all = case.read(ReadRecordsRequest::default()).await.unwrap(); + assert_eq!(texts(&all), ["1", "2"]); + assert_eq!(all.records[0].offset, first.first_offset); + assert_eq!(all.next_offset, first.last_offset + 1); + let from = case + .read(ReadRecordsRequest { + from_offset: first.last_offset, + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(texts(&from), ["2"]); + let next = case.describe().await.unwrap().last_offset + 1; + let none = case + .read(ReadRecordsRequest { + from_offset: next, + ..Default::default() + }) + .await + .unwrap(); + assert!(none.records.is_empty()); + assert_eq!(none.next_offset, next); + let end = case + .read(ReadRecordsRequest { + start: StreamReadStart::End as i32, + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(end.next_offset, next); + let error = case + .read(ReadRecordsRequest { + from_offset: next + 1, + ..Default::default() + }) + .await + .unwrap_err(); + assert_eq!( + (error.code(), reason(&error).as_str()), + (Code::OutOfRange, "stream_offset") + ); + let error = case + .read(ReadRecordsRequest { + from_offset: -1, + ..Default::default() + }) + .await + .unwrap_err(); + assert_eq!(reason(&error), "stream_invalid"); + } +} + +/// 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 supersession = superseded_of(superseded); + assert_eq!(supersession.producer_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 { + let error = case.read(ReadRecordsRequest::default()).await.unwrap_err(); + assert_eq!( + (error.code(), reason(&error).as_str()), + (Code::NotFound, "stream_not_found") + ); + let started = Instant::now(); + let error = case + .read(ReadRecordsRequest { + create_wait: millis(200), + ..Default::default() + }) + .await + .unwrap_err(); + assert_eq!(error.code(), Code::NotFound); + assert!(started.elapsed() >= Duration::from_millis(200)); + let ext = case.ext.clone(); + let (namespace, stream) = (case.namespace.clone(), case.stream.clone()); + let creator = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(150)).await; + let request = CreateStreamRequest { + namespace, + stream_id: stream, + ..Default::default() + }; + call::<_, CreateStreamResponse>(&ext, "CreateStream", request).await + }); + let read = case + .read(ReadRecordsRequest { + create_wait: millis(3000), + ..Default::default() + }) + .await + .unwrap(); + assert!(read.records.is_empty()); + creator.await.unwrap().unwrap(); + } +} + +/// A quiet read ends empty after its wait, and a long poll wakes on an append. +async fn a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let first = case.append(client(1), 1, vec![record("1")]).await.unwrap(); + let from = first.last_offset + 1; + let started = Instant::now(); + let quiet = case + .read(ReadRecordsRequest { + from_offset: from, + wait: millis(200), + ..Default::default() + }) + .await + .unwrap(); + assert!(started.elapsed() >= Duration::from_millis(200)); + assert!(started.elapsed() < Duration::from_secs(2)); + assert!(quiet.records.is_empty()); + assert_eq!(quiet.next_offset, from); + assert!(!quiet.done); + let ext = case.ext.clone(); + let (namespace, stream) = (case.namespace.clone(), case.stream.clone()); + let writer = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(150)).await; + let request = AppendRecordsRequest { + namespace, + stream: Some(StreamRef { + id: stream, + ..Default::default() + }), + producer: Some(client(1)), + sequence: 2, + records: vec![record("2")], + ..Default::default() + }; + call::<_, AppendRecordsResponse>(&ext, "AppendRecords", request).await + }); + let woke = case + .read(ReadRecordsRequest { + from_offset: from, + wait: millis(5000), + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(texts(&woke), ["2"]); + writer.await.unwrap().unwrap(); + } +} + +/// A close ends every read with its close Record and refuses later appends. +async fn a_close_ends_every_read_with_its_record() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + 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!(read.done); + assert_eq!( + close_of(&read.records[1]).result.as_ref().unwrap().data, + b"done" + ); + let again = case + .read(ReadRecordsRequest { + from_offset: read.next_offset, + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(texts(&again), [""]); + assert!(again.done); + assert_eq!( + case.describe().await.unwrap().status, + StreamStatus::Closed as i32 + ); + let error = case + .append(client(1), 2, vec![record("2")]) + .await + .unwrap_err(); + assert_eq!( + (error.code(), reason(&error).as_str()), + (Code::FailedPrecondition, "stream_closed") + ); + } +} + /// An unknown method fails with UNIMPLEMENTED. async fn an_unknown_method_is_unimplemented() { for case in cases_for(Needs::CALLS).await { @@ -307,6 +643,456 @@ 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), 0, 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() { + for case in cases_for(Needs::SHORT_TIMES).await { + case.create(millis(300)).await.unwrap(); + let first = case.append(client(1), 1, vec![record("1")]).await.unwrap(); + case.close(client(1)).await.unwrap(); + tokio::time::sleep(Duration::from_millis(500)).await; + let describe = case.describe().await.unwrap(); + assert_eq!(describe.status, StreamStatus::Expired as i32); + for request in [ + ReadRecordsRequest::default(), + ReadRecordsRequest { + from_offset: first.first_offset, + ..Default::default() + }, + ReadRecordsRequest { + from_offset: describe.last_offset + 1, + ..Default::default() + }, + ] { + let started = Instant::now(); + let error = case + .read(ReadRecordsRequest { + wait: millis(1500), + ..request + }) + .await + .unwrap_err(); + assert_eq!(reason(&error), "stream_expired"); + assert!(started.elapsed() < Duration::from_secs(1)); + } + let last = case + .read(ReadRecordsRequest { + last: 5, + ..Default::default() + }) + .await + .unwrap_err(); + assert_eq!(reason(&last), "stream_expired"); + } +} + +/// 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(), + stream_id: case.stream.clone(), + retention: millis(300), + owner: Some(workflow_owner(run)), + r#ref: owned_ref(case), + ..Default::default() + }, + ) + .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; + let first = case + .append(client(1), 1, vec![record("a"), record("b")]) + .await + .unwrap(); + tokio::time::sleep(Duration::from_millis(400)).await; + let behind = ReadRecordsRequest { + from_offset: first.first_offset + 1, + wait: millis(1500), + ..Default::default() + }; + let started = Instant::now(); + let error = case.read(behind.clone()).await.unwrap_err(); + assert_eq!(reason(&error), "stream_expired"); + assert!(started.elapsed() < Duration::from_secs(1)); + case.append(client(1), 2, vec![record("c")]).await.unwrap(); + let error = case.read(behind).await.unwrap_err(); + assert_eq!(reason(&error), "stream_expired"); + let read = case.read(ReadRecordsRequest::default()).await.unwrap(); + assert_eq!(texts(&read), ["c"]); + } +} + +/// A close as the first write 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; + 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(), + stream: owned_ref(&case), + producer: Some(workflow_owner(&run)), + ..Default::default() + }; + let _: CloseStreamResponse = call(&case.ext, "CloseStream", close).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 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()), (3, other)] { + let request = AppendRecordsRequest { + namespace: case.namespace.clone(), + stream, + producer: Some(client(1)), + sequence, + records: vec![record(&sequence.to_string())], + ..Default::default() + }; + let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", request).await.unwrap(); + } + let read = |stream: Option| ReadRecordsRequest { + namespace: case.namespace.clone(), + stream, + ..Default::default() + }; + let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.stream())) + .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") { + let unique = unique(); + let case = Case { + ext, + namespace: format!("ns-{unique}"), + stream: format!("s-{unique}"), + gives: Needs::ALL, + }; + let created = case.create(None).await.unwrap(); + let stream = created.stream.unwrap(); + assert_eq!(stream.id, case.stream); + assert_eq!(stream.backend, "redis-a"); + // A ref with any backend, or none, reaches the same Stream. + let named = |backend: &str| { + 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"))] { + let request = AppendRecordsRequest { + namespace: case.namespace.clone(), + stream, + producer: Some(client(1)), + sequence, + records: vec![record(&sequence.to_string())], + ..Default::default() + }; + let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", request).await.unwrap(); + } + let read = ReadRecordsRequest { + namespace: case.namespace.clone(), + stream: named("other"), + ..Default::default() + }; + let read: ReadRecordsResponse = call(&case.ext, "ReadRecords", read).await.unwrap(); + assert_eq!(texts(&read), ["1", "2", "3"]); + } + // With no backend in the config, the ref has none, and lang reads it as the default. + for case in cases_for(Needs::OWN_CONFIG).await { + let created = case.create(None).await.unwrap(); + assert_eq!(created.stream.unwrap().backend, ""); + } +} + +fn workflow_owner(run: &str) -> Producer { + Producer { + kind: StreamProducerKind::Workflow as i32, + id: crate::ids::workflow_producer_id(run), + run_id: run.to_string(), + ..Default::default() + } +} + +/// 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() + }) +} + +/// 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 { + Producer { + kind: StreamProducerKind::LocalActivity as i32, + id: crate::ids::local_activity_producer_id(run, "call-1"), + epoch: Some(ProducerEpoch { + task_started_event_id: started, + task_attempt: 1, + task_started_time: Some(Timestamp { seconds, nanos: 0 }), + attempt: 1, + ..Default::default() + }), + run_id: run.to_string(), + } +} + +/// 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(), + stream_id: case.stream.clone(), + owner: Some(workflow_owner(&run)), + r#ref: owned_ref(&case), + ..Default::default() + }, + ) + .await + .unwrap(); + let write = |producer: Producer, text: &str| AppendRecordsRequest { + namespace: case.namespace.clone(), + stream: case.stream(), + producer: Some(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 close Record gives its status. A caller can't close as terminated or timed out, nor give a +/// status that its failure contradicts, nor a close Record over the Record limit. +async fn a_close_record_gives_its_status_and_a_caller_cant_terminate() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let close = |status: StreamCloseStatus, failure: Option| CloseStreamRequest { + namespace: case.namespace.clone(), + stream: case.stream(), + producer: Some(client(1)), + failure, + status: status as i32, + ..Default::default() + }; + let failure = Failure { + message: "boom".to_string(), + ..Default::default() + }; + for request in [ + close(StreamCloseStatus::Terminated, None), + close(StreamCloseStatus::TimedOut, None), + close(StreamCloseStatus::Completed, Some(failure.clone())), + close(StreamCloseStatus::Failed, None), + ] { + let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", request) + .await + .unwrap_err(); + assert_eq!( + (error.code(), reason(&error).as_str()), + (Code::InvalidArgument, "stream_invalid") + ); + } + let huge = CloseStreamRequest { + result: Some(Payload { + data: vec![0; crate::store::MAX_RECORD_BYTES], + ..Default::default() + }), + ..close(StreamCloseStatus::Unspecified, None) + }; + let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", huge) + .await + .unwrap_err(); + assert_eq!(reason(&error), "stream_too_large"); + let failed = close(StreamCloseStatus::Unspecified, Some(failure)); + call::<_, CloseStreamResponse>(&case.ext, "CloseStream", failed) + .await + .unwrap(); + let read = case.read(ReadRecordsRequest::default()).await.unwrap(); + let record = read.records.last().unwrap(); + assert_eq!(close_of(record).status, StreamCloseStatus::Failed as i32); + assert_eq!(close_of(record).failure.as_ref().unwrap().message, "boom"); + } +} + +/// A last read gives the newest Records oldest first, skips withdrawn ones, and ends with the +/// close Record of a closed Stream. +async fn a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_ones() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let last = |n: i32| ReadRecordsRequest { + last: n, + ..Default::default() + }; + 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(); + } + assert_eq!( + texts(&case.read(last(3)).await.unwrap()), + ["c2", "c3", "a2"] + ); + let read = case.read(last(2)).await.unwrap(); + assert_eq!(texts(&read), ["c3", "a2"]); + assert!(!read.done); + let newest = case.describe().await.unwrap().last_offset; + assert_eq!(read.next_offset, newest + 1); + let error = case + .read(ReadRecordsRequest { + from_offset: 1, + ..last(1) + }) + .await + .unwrap_err(); + assert_eq!(reason(&error), "stream_invalid"); + case.close(client(1)).await.unwrap(); + let read = case.read(last(1)).await.unwrap(); + assert_eq!(texts(&read), ["a2", ""]); + assert!(read.done); + } +} + +/// A page stays under `max_bytes`, and a larger Record is still returned alone. +async fn a_page_keeps_under_max_bytes_and_a_larger_record_comes_alone() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let big = "x".repeat(1000); + let records = vec![record("a"), record("b"), record(&big), record("c")]; + case.append(client(1), 1, records).await.unwrap(); + let capped = |from_offset| ReadRecordsRequest { + from_offset, + max_bytes: 200, + ..Default::default() + }; + let first = case.read(capped(0)).await.unwrap(); + assert_eq!(texts(&first), ["a", "b"]); + let second = case.read(capped(first.next_offset)).await.unwrap(); + assert_eq!(texts(&second), [big]); + let third = case.read(capped(second.next_offset)).await.unwrap(); + assert_eq!(texts(&third), ["c"]); + } +} + /// The cases of the suite, in file order. pub const CASES: &[super::Case] = &[ case!(a_create_returns_the_stream_that_exists_whatever_its_retention), @@ -314,5 +1100,20 @@ pub const CASES: &[super::Case] = &[ 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!(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_its_record), 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_rerun_local_activity_supersedes_its_first_attempt), + case!(a_close_record_gives_its_status_and_a_caller_cant_terminate), + case!(a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_ones), + case!(a_page_keeps_under_max_bytes_and_a_larger_record_comes_alone), ]; diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs new file mode 100644 index 000000000..f3e29ab9f --- /dev/null +++ b/crates/streams/tests/scans.rs @@ -0,0 +1,235 @@ +//! How a read scans the log, seen through a memory Store that can trim under a read. + +use prost::Message; +use std::{ + sync::{ + Arc, + atomic::{AtomicBool, AtomicUsize, Ordering}, + }, + time::Duration, +}; +use temporalio_common::protos::temporal::api::common::v1::Payload; +use temporalio_streams::{ + MemoryStreamRuntime, StreamError, StreamResult, StreamRuntime, + ids::client_producer_id, + memory::MemoryStore, + proto::{ + AppendRecord, AppendRecordsRequest, AppendRecordsResponse, CreateStreamRequest, + CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, + StreamErrorDetails, StreamErrorReason, StreamProducerKind, StreamRef, + }, + store::{Appended, Batch, Capabilities, Created, Entry, Owner, Store, StreamState, Withdrawal}, +}; +use tokio::time::Instant; + +const NAMESPACE: &str = "ns"; +const STREAM: &str = "s"; + +/// The memory Store. Once armed, its next read that asks for log entries finds its first entry +/// trimmed, as a trim just before that read leaves it. It counts the entries that reads return. +struct Trimming { + inner: MemoryStore, + trim_next: AtomicBool, + pulled: AtomicUsize, +} + +#[async_trait::async_trait] +impl Store for Trimming { + fn capabilities(&self) -> Capabilities { + self.inner.capabilities() + } + + fn default_retention(&self) -> Duration { + self.inner.default_retention() + } + + async fn create( + &self, + namespace: &str, + stream: &StreamRef, + retention: Option, + owner: Owner, + withdrawal: &Withdrawal<'_>, + ) -> StreamResult { + self.inner + .create(namespace, stream, retention, owner, withdrawal) + .await + } + + async fn append( + &self, + namespace: &str, + stream: &StreamRef, + batch: &Batch<'_>, + ) -> StreamResult { + self.inner.append(namespace, stream, batch).await + } + + async fn read( + &self, + namespace: &str, + stream_id: &str, + from: i64, + count: usize, + ) -> StreamResult<(StreamState, Vec)> { + let (mut state, mut entries) = self.inner.read(namespace, stream_id, from, count).await?; + if count > 0 && !entries.is_empty() && self.trim_next.swap(false, Ordering::Relaxed) { + state.trimmed = entries.remove(0).offset; + } + self.pulled.fetch_add(entries.len(), Ordering::Relaxed); + Ok((state, entries)) + } + + async fn wait_after( + &self, + namespace: &str, + stream_id: &str, + after: i64, + deadline: Instant, + ) -> StreamResult { + self.inner + .wait_after(namespace, stream_id, after, deadline) + .await + } +} + +fn trimming() -> (Arc, Arc) { + let store = Arc::new(Trimming { + inner: MemoryStore::new(Duration::from_secs(3600), Duration::from_secs(7200)), + trim_next: AtomicBool::new(false), + pulled: AtomicUsize::new(0), + }); + let ext = MemoryStreamRuntime::with_store(store.clone(), None); + (store, ext) +} + +async fn call( + ext: &StreamRuntime, + method: &str, + request: Req, +) -> Resp { + try_call(ext, method, request).await.unwrap() +} + +async fn try_call( + ext: &StreamRuntime, + method: &str, + request: Req, +) -> Result { + let bytes = ext.call(method, &request.encode_to_vec()).await?; + Ok(Resp::decode(bytes.as_slice()).unwrap()) +} + +async fn create(ext: &StreamRuntime) { + let _: CreateStreamResponse = call( + ext, + "CreateStream", + CreateStreamRequest { + namespace: NAMESPACE.to_string(), + stream_id: STREAM.to_string(), + ..Default::default() + }, + ) + .await; +} + +fn client() -> Producer { + Producer { + kind: StreamProducerKind::Client as i32, + id: client_producer_id("me", "p"), + epoch: Some(ProducerEpoch { + attempt: 1, + ..Default::default() + }), + run_id: String::new(), + } +} + +fn stream() -> Option { + Some(StreamRef { + id: STREAM.to_string(), + ..Default::default() + }) +} + +async fn append(ext: &StreamRuntime, sequence: i64, count: usize) -> AppendRecordsResponse { + append_sized(ext, sequence, count, 0).await +} + +/// Appends `count` Records, each with `size` bytes after its number. +async fn append_sized( + ext: &StreamRuntime, + sequence: i64, + count: usize, + size: usize, +) -> AppendRecordsResponse { + let records = (0..count) + .map(|i| AppendRecord { + body: Some(Payload { + data: format!("{i}{}", "x".repeat(size)).into_bytes(), + ..Default::default() + }), + }) + .collect(); + call( + ext, + "AppendRecords", + AppendRecordsRequest { + namespace: NAMESPACE.to_string(), + stream: stream(), + producer: Some(client()), + sequence, + records, + ..Default::default() + }, + ) + .await +} + +/// A trim between a read's state and its log read fails a read from an offset as expired, so that +/// the Reader always sees the gap. A read from the start skips the trimmed Records. +#[tokio::test] +async fn a_trim_under_a_read_from_an_offset_fails_it_as_expired() { + let (store, ext) = trimming(); + create(&ext).await; + let first = append(&ext, 1, 3).await; + let read = |from_offset| ReadRecordsRequest { + namespace: NAMESPACE.to_string(), + stream: stream(), + from_offset, + ..Default::default() + }; + store.trim_next.store(true, Ordering::Relaxed); + let error = try_call::<_, ReadRecordsResponse>(&ext, "ReadRecords", read(first.first_offset)) + .await + .unwrap_err(); + let details = StreamErrorDetails::decode(error.details().as_slice()).unwrap(); + assert_eq!(details.reason(), StreamErrorReason::Expired); + store.trim_next.store(true, Ordering::Relaxed); + let from_start: ReadRecordsResponse = call(&ext, "ReadRecords", read(0)).await; + assert_eq!(from_start.records.len(), 2); +} + +/// A read with a byte cap reads about the entries that its page holds from the Store, so a small +/// cap on large Records doesn't read a whole page of them. +#[tokio::test] +async fn a_capped_read_pulls_about_its_page_from_the_store() { + let (store, ext) = trimming(); + create(&ext).await; + append_sized(&ext, 1, 50, 1000).await; + store.pulled.store(0, Ordering::Relaxed); + let page: ReadRecordsResponse = call( + &ext, + "ReadRecords", + ReadRecordsRequest { + namespace: NAMESPACE.to_string(), + stream: stream(), + max_bytes: 2500, + ..Default::default() + }, + ) + .await; + assert_eq!(page.records.len(), 2); + let pulled = store.pulled.load(Ordering::Relaxed); + assert!(pulled <= 4, "{pulled}"); +} From 14a2101c15aad904974f0c2e47c5ad4e0070521a Mon Sep 17 00:00:00 2001 From: Mohammad Dashti Date: Fri, 30 Oct 2026 00:31:05 -0700 Subject: [PATCH 02/11] Stopped exporting MAX_RECORDS, which no consumer reads. MAX_WAIT stays public, since the native backend reads it. --- crates/streams/src/lib.rs | 2 +- crates/streams/src/service.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/streams/src/lib.rs b/crates/streams/src/lib.rs index d99949418..2e1381378 100644 --- a/crates/streams/src/lib.rs +++ b/crates/streams/src/lib.rs @@ -23,7 +23,7 @@ pub mod testing; pub use error::{Reason, StreamError, StreamResult}; pub use runtime::{MemoryStreamRuntime, StoreFactory, StreamRuntime}; -pub use service::{MAX_RECORDS, MAX_WAIT}; +pub use service::MAX_WAIT; /// The protos that lang and the crate exchange. pub mod proto { diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index 44856eb4d..409c3726d 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -26,7 +26,7 @@ use tokio::time::Instant; /// The longest wait of one read, for new Records and for the create. pub const MAX_WAIT: Duration = Duration::from_secs(60); /// The maximum number of Records in one read. -pub const MAX_RECORDS: usize = 1000; +const MAX_RECORDS: usize = 1000; /// The maximum number of log entries that a read of the last Records scans. const MAX_SCAN: usize = 10 * MAX_RECORDS; /// The most entry bytes that a read of the last Records scans. From e1bd1447955473095892bd5b08018ac541a203f4 Mon Sep 17 00:00:00 2001 From: Mohammad Dashti Date: Fri, 30 Oct 2026 10:32:24 -0700 Subject: [PATCH 03/11] Matched the close status test to the failure rule. The test closes once with a plain failure and once with an Activity failure whose cause is a cancel. The expiry case sends a negative sequence, since zero takes the next one. --- crates/streams/src/testing/service.rs | 100 +++++++++++++------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index cbbb6b077..2423b5b58 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -22,7 +22,10 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; -use temporalio_common::protos::temporal::api::{common::v1::Payload, failure::v1::Failure}; +use temporalio_common::protos::temporal::api::{ + common::v1::Payload, + failure::v1::{Failure, failure::FailureInfo}, +}; use tonic::Code; struct Case { @@ -684,7 +687,7 @@ async fn a_read_of_an_expired_open_stream_fails_instead_of_waiting() { assert_eq!(reason(&error), "stream_expired"); assert!(started.elapsed() < Duration::from_secs(2)); } - let error = case.append(client(1), 0, vec![]).await.unwrap_err(); + let error = case.append(client(1), -1, vec![]).await.unwrap_err(); assert_eq!(reason(&error), "stream_invalid"); } } @@ -992,56 +995,55 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { } } -/// A close Record gives its status. A caller can't close as terminated or timed out, nor give a -/// status that its failure contradicts, nor a close Record over the Record limit. -async fn a_close_record_gives_its_status_and_a_caller_cant_terminate() { - for case in cases_for(Needs::CALLS).await { - case.create(None).await.unwrap(); - let close = |status: StreamCloseStatus, failure: Option| CloseStreamRequest { - namespace: case.namespace.clone(), - stream: case.stream(), - producer: Some(client(1)), - failure, - status: status as i32, - ..Default::default() - }; - let failure = Failure { - message: "boom".to_string(), +/// A close Record takes its status from the failure. A failure is canceled when it is a cancel, or +/// an Activity, child Workflow or Nexus Operation failure whose cause is a cancel, and failed +/// otherwise. A close Record over the Record limit is refused. +async fn a_close_record_takes_its_status_from_the_failure() { + let canceled_activity = Failure { + message: "boom".to_string(), + failure_info: Some(FailureInfo::ActivityFailureInfo(Default::default())), + cause: Some(Box::new(Failure { + failure_info: Some(FailureInfo::CanceledFailureInfo(Default::default())), ..Default::default() - }; - for request in [ - close(StreamCloseStatus::Terminated, None), - close(StreamCloseStatus::TimedOut, None), - close(StreamCloseStatus::Completed, Some(failure.clone())), - close(StreamCloseStatus::Failed, None), - ] { - let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", request) + })), + ..Default::default() + }; + let other = Failure { + message: "boom".to_string(), + ..Default::default() + }; + for (failure, status) in [ + (other, StreamCloseStatus::Failed), + (canceled_activity, StreamCloseStatus::Canceled), + ] { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let close = CloseStreamRequest { + namespace: case.namespace.clone(), + stream: case.stream(), + producer: Some(client(1)), + failure: Some(failure.clone()), + ..Default::default() + }; + let huge = CloseStreamRequest { + result: Some(Payload { + data: vec![0; crate::store::MAX_RECORD_BYTES], + ..Default::default() + }), + ..close.clone() + }; + let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", huge) .await .unwrap_err(); - assert_eq!( - (error.code(), reason(&error).as_str()), - (Code::InvalidArgument, "stream_invalid") - ); + assert_eq!(reason(&error), "stream_too_large"); + call::<_, CloseStreamResponse>(&case.ext, "CloseStream", close) + .await + .unwrap(); + let read = case.read(ReadRecordsRequest::default()).await.unwrap(); + let record = read.records.last().unwrap(); + assert_eq!(close_of(record).status, status as i32); + assert_eq!(close_of(record).failure.as_ref().unwrap().message, "boom"); } - let huge = CloseStreamRequest { - result: Some(Payload { - data: vec![0; crate::store::MAX_RECORD_BYTES], - ..Default::default() - }), - ..close(StreamCloseStatus::Unspecified, None) - }; - let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", huge) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_too_large"); - let failed = close(StreamCloseStatus::Unspecified, Some(failure)); - call::<_, CloseStreamResponse>(&case.ext, "CloseStream", failed) - .await - .unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - let record = read.records.last().unwrap(); - assert_eq!(close_of(record).status, StreamCloseStatus::Failed as i32); - assert_eq!(close_of(record).failure.as_ref().unwrap().message, "boom"); } } @@ -1136,7 +1138,7 @@ pub const CASES: &[super::Case] = &[ 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_rerun_local_activity_supersedes_its_first_attempt), - case!(a_close_record_gives_its_status_and_a_caller_cant_terminate), + case!(a_close_record_takes_its_status_from_the_failure), case!(a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_ones), case!(a_page_keeps_under_max_bytes_and_a_larger_record_comes_alone), ]; From 4648317fec9ac1ac5b8c6a71f4acc08b2d95bdf3 Mon Sep 17 00:00:00 2001 From: Mohammad Dashti Date: Sun, 1 Nov 2026 03:01:45 -0700 Subject: [PATCH 04/11] Renamed Supersession to Superseded, and StreamRef to StreamHandle. --- crates/streams/src/service.rs | 20 ++++---- crates/streams/src/testing/service.rs | 67 +++++++++++++-------------- crates/streams/tests/scans.rs | 20 ++++---- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index 08fbc9256..00311dc2c 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -11,7 +11,7 @@ use crate::{ AppendRecordsRequest, AppendRecordsResponse, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, Producer, ReadRecordsRequest, ReadRecordsResponse, StreamHandle, StreamProducerKind, - StreamReadStart, StreamRecord, StreamStatus, Supersession, stream_record::Item, + StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, stream_record::Item, }, record::{caller_status, close_record, data_records, superseded}, store::{ @@ -336,7 +336,7 @@ impl StreamService { &self, request: ReadRecordsRequest, ) -> StreamResult { - let id = stream_id(request.stream.as_ref())?.to_string(); + let id = stream_id(request.handle.as_ref())?.to_string(); let namespace = request.namespace.as_str(); if request.from_offset < 0 { return Err(StreamError::invalid("The offset is below zero")); @@ -423,7 +423,7 @@ impl StreamService { request: ReadRecordsRequest, max_bytes: usize, ) -> StreamResult { - let id = stream_id(request.stream.as_ref())?.to_string(); + let id = stream_id(request.handle.as_ref())?.to_string(); let namespace = request.namespace.as_str(); let count = (request.last as usize).min(MAX_RECORDS); let create_wait = duration(request.create_wait) @@ -441,7 +441,7 @@ impl StreamService { check_expiry(&state, 0)?; let retained = state.retained_from(); let (mut picked, mut close) = (vec![], None); - let mut withdrawals: Vec = 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 @@ -465,7 +465,7 @@ impl StreamService { before = entry.offset - 1; scanned += 1; scanned_bytes += entry.record.as_ref().map_or(0, Vec::len) - + entry.supersession.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)?; @@ -486,9 +486,9 @@ impl StreamService { } } } - if let Some(supersession) = entry.supersession { + if let Some(superseded) = entry.superseded { withdrawals.push( - Supersession::decode(supersession.as_slice()) + StreamSuperseded::decode(superseded.as_slice()) .map_err(|error| StreamError::storage(error.to_string()))?, ); } @@ -587,12 +587,12 @@ impl StreamService { continue; } let mut items = vec![]; - if let Some(bytes) = entry.supersession { - let supersession = Supersession::decode(bytes.as_slice()) + if let Some(bytes) = entry.superseded { + let superseded = StreamSuperseded::decode(bytes.as_slice()) .map_err(|error| StreamError::storage(error.to_string()))?; items.push(StreamRecord { offset: entry.offset, - item: Some(Item::Superseded(supersession)), + item: Some(Item::Superseded(superseded)), }); } if let Some(bytes) = entry.record { diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 32059b68c..00940e845 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -13,7 +13,7 @@ use crate::{ CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, StreamClose, StreamCloseStatus, StreamErrorDetails, StreamHandle, StreamProducerKind, - StreamReadStart, StreamRecord, StreamStatus, Supersession, stream_record::Item, + StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, stream_record::Item, }, }; use prost::Message; @@ -191,7 +191,7 @@ impl Case { "ReadRecords", ReadRecordsRequest { namespace: self.namespace.clone(), - stream: self.stream(), + handle: self.handle(), ..request }, ) @@ -204,7 +204,7 @@ impl Case { "DescribeStream", DescribeStreamRequest { namespace: self.namespace.clone(), - stream: self.stream(), + handle: self.handle(), }, ) .await @@ -216,7 +216,7 @@ impl Case { "CloseStream", CloseStreamRequest { namespace: self.namespace.clone(), - stream: self.stream(), + handle: self.handle(), producer: Some(producer), result: Some(Payload { data: b"done".to_vec(), @@ -242,9 +242,9 @@ fn texts(response: &ReadRecordsResponse) -> Vec { .collect() } -fn superseded_of(record: &StreamRecord) -> &Supersession { +fn superseded_of(record: &StreamRecord) -> &StreamSuperseded { match &record.item { - Some(Item::Superseded(supersession)) => supersession, + Some(Item::Superseded(superseded)) => superseded, other => panic!("not a Superseded item: {other:?}"), } } @@ -515,8 +515,7 @@ async fn a_higher_attempt_puts_its_superseded_item_before_its_batch() { let superseded = &all.records[1]; assert!(all.records[0].offset < superseded.offset); assert!(superseded.offset <= all.records[2].offset); - let supersession = superseded_of(superseded); - assert_eq!(supersession.producer_id, activity(2, 20).id); + assert_eq!(superseded_of(superseded).producer_id, activity(2, 20).id); let mut from_offset = 0; for expected in [&["old"][..], &["", "new1"], &["new2"]] { let page = case @@ -600,7 +599,7 @@ async fn a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append( tokio::time::sleep(Duration::from_millis(150)).await; let request = AppendRecordsRequest { namespace, - stream: Some(StreamRef { + handle: Some(StreamHandle { id: stream, ..Default::default() }), @@ -747,7 +746,7 @@ async fn create_owned_short(case: &Case, run: &str) { stream_id: case.stream.clone(), retention: millis(300), owner: Some(workflow_owner(run)), - r#ref: owned_ref(case), + stream_handle: owned_handle(case), ..Default::default() }, ) @@ -795,7 +794,7 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { tokio::time::sleep(Duration::from_millis(400)).await; let close = CloseStreamRequest { namespace: case.namespace.clone(), - stream: owned_ref(&case), + handle: owned_handle(&case), producer: Some(workflow_owner(&run)), ..Default::default() }; @@ -809,25 +808,25 @@ 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() { +/// A handle with an owner and one without reach the same Stream. +async fn a_handle_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 { + let owned = Some(StreamHandle { 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 { + let other = Some(StreamHandle { id: case.stream.clone(), owner: super::support::workflow_execution("someone-else", ""), ..Default::default() }); - for (sequence, stream) in [(1, owned.clone()), (2, case.stream()), (3, other)] { + for (sequence, stream) in [(1, owned.clone()), (2, case.handle()), (3, other)] { let request = AppendRecordsRequest { namespace: case.namespace.clone(), - stream, + handle: stream, producer: Some(client(1)), sequence, records: vec![record(&sequence.to_string())], @@ -835,12 +834,12 @@ async fn a_ref_with_an_owner_and_one_without_reach_the_same_stream() { }; let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", request).await.unwrap(); } - let read = |stream: Option| ReadRecordsRequest { + let read = |handle: Option| ReadRecordsRequest { namespace: case.namespace.clone(), - stream, + handle, ..Default::default() }; - let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.stream())) + let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.handle())) .await .unwrap(); let with_owner: ReadRecordsResponse = @@ -861,12 +860,12 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { gives: Needs::ALL, }; let created = case.create(None).await.unwrap(); - let stream = created.stream.unwrap(); + let stream = created.handle.unwrap(); assert_eq!(stream.id, case.stream); assert_eq!(stream.backend, "redis-a"); - // A ref with any backend, or none, reaches the same Stream. + // A handle with any backend, or none, reaches the same Stream. let named = |backend: &str| { - Some(StreamRef { + Some(StreamHandle { id: case.stream.clone(), backend: backend.to_string(), ..Default::default() @@ -875,7 +874,7 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { for (sequence, stream) in [(1, named("redis-a")), (2, named("")), (3, named("other"))] { let request = AppendRecordsRequest { namespace: case.namespace.clone(), - stream, + handle: stream, producer: Some(client(1)), sequence, records: vec![record(&sequence.to_string())], @@ -885,16 +884,16 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { } let read = ReadRecordsRequest { namespace: case.namespace.clone(), - stream: named("other"), + handle: named("other"), ..Default::default() }; let read: ReadRecordsResponse = call(&case.ext, "ReadRecords", read).await.unwrap(); assert_eq!(texts(&read), ["1", "2", "3"]); } - // With no backend in the config, the ref has none, and lang reads it as the default. + // With no backend in the config, the handle has none, and lang reads it as the default. for case in cases_for(Needs::OWN_CONFIG).await { let created = case.create(None).await.unwrap(); - assert_eq!(created.stream.unwrap().backend, ""); + assert_eq!(created.handle.unwrap().backend, ""); } } @@ -907,9 +906,9 @@ fn workflow_owner(run: &str) -> Producer { } } -/// 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 { +/// The case's handle with the owner of a made-up run chain, which a create with an owner needs. +fn owned_handle(case: &Case) -> Option { + Some(StreamHandle { id: case.stream.clone(), owner: super::support::workflow_execution(&format!("{}-wf", case.stream), "first-run"), ..Default::default() @@ -944,7 +943,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { namespace: case.namespace.clone(), stream_id: case.stream.clone(), owner: Some(workflow_owner(&run)), - r#ref: owned_ref(&case), + stream_handle: owned_handle(&case), ..Default::default() }, ) @@ -952,7 +951,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { .unwrap(); let write = |producer: Producer, text: &str| AppendRecordsRequest { namespace: case.namespace.clone(), - stream: case.stream(), + handle: case.handle(), producer: Some(producer), sequence: 1, records: vec![record(text)], @@ -1021,7 +1020,7 @@ async fn a_close_record_takes_its_status_from_the_failure() { case.create(None).await.unwrap(); let close = CloseStreamRequest { namespace: case.namespace.clone(), - stream: case.stream(), + handle: case.handle(), producer: Some(client(1)), failure: Some(failure.clone()), ..Default::default() @@ -1136,7 +1135,7 @@ pub const CASES: &[super::Case] = &[ 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_handle_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_rerun_local_activity_supersedes_its_first_attempt), case!(a_close_record_takes_its_status_from_the_failure), diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index 03b900afa..73974f44a 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -16,7 +16,7 @@ use temporalio_streams::{ proto::{ AppendRecord, AppendRecordsRequest, AppendRecordsResponse, CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, - StreamErrorDetails, StreamErrorReason, StreamProducerKind, StreamRef, + StreamErrorDetails, StreamErrorReason, StreamHandle, StreamProducerKind, }, store::{Appended, Batch, Capabilities, Created, Entry, Owner, Store, StreamState, Withdrawal}, }; @@ -42,23 +42,23 @@ impl Store for Trimming { async fn create( &self, namespace: &str, - stream: &StreamRef, + handle: &StreamHandle, retention: Option, owner: Owner, withdrawal: &Withdrawal<'_>, ) -> StreamResult { self.inner - .create(namespace, stream, retention, owner, withdrawal) + .create(namespace, handle, retention, owner, withdrawal) .await } async fn append( &self, namespace: &str, - stream: &StreamRef, + handle: &StreamHandle, batch: &Batch<'_>, ) -> StreamResult { - self.inner.append(namespace, stream, batch).await + self.inner.append(namespace, handle, batch).await } async fn read( @@ -141,8 +141,8 @@ fn client() -> Producer { } } -fn stream() -> Option { - Some(StreamRef { +fn handle() -> Option { + Some(StreamHandle { id: STREAM.to_string(), ..Default::default() }) @@ -172,7 +172,7 @@ async fn append_sized( "AppendRecords", AppendRecordsRequest { namespace: NAMESPACE.to_string(), - stream: stream(), + handle: handle(), producer: Some(client()), sequence, records, @@ -191,7 +191,7 @@ async fn a_trim_under_a_read_from_an_offset_fails_it_as_expired() { let first = append(&ext, 1, 3).await; let read = |from_offset| ReadRecordsRequest { namespace: NAMESPACE.to_string(), - stream: stream(), + handle: handle(), from_offset, ..Default::default() }; @@ -219,7 +219,7 @@ async fn a_capped_read_pulls_about_its_page_from_the_store() { "ReadRecords", ReadRecordsRequest { namespace: NAMESPACE.to_string(), - stream: stream(), + handle: handle(), max_bytes: 2500, ..Default::default() }, From a1b958c2a8f5c0dfb913f89e2c243db5d973427e Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Wed, 4 Nov 2026 18:03:04 -0700 Subject: [PATCH 05/11] Built the read suite's Producers from their epoch cases. The read past the end fails as stream_offset_too_high, and each create names its Stream by its handle. The suite keeps one close helper and one Workflow Producer helper. --- crates/streams/src/service.rs | 5 +- crates/streams/src/testing/service.rs | 78 ++++++++++----------------- crates/streams/tests/scans.rs | 23 ++++---- 3 files changed, 40 insertions(+), 66 deletions(-) diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index 9f35c780a..bd94df804 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -11,8 +11,7 @@ use crate::{ AppendRecordsRequest, AppendRecordsResponse, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, Producer, ReadRecordsRequest, ReadRecordsResponse, StreamHandle, StreamIdConflictPolicy, - StreamProducerKind, StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, - stream_record::Item, + StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, stream_record::Item, }, record::{caller_status, close_record, data_records, superseded}, store::{ @@ -396,7 +395,7 @@ impl StreamService { if from > state.newest + 1 { let next = state.newest + 1; return Err(StreamError::new( - Reason::Offset, + Reason::OffsetTooHigh, format!( "The offset {from} is past the newest offset + 1, {next}. A failover may have \ lost the newest Records. Read from {next}, or read again once newer Records \ diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 3968e01db..5ca286ae1 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -13,9 +13,9 @@ use crate::{ CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, StreamClose, StreamCloseStatus, - StreamErrorDetails, StreamHandle, StreamIdConflictPolicy, StreamProducerKind, - StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, WorkflowTaskEpoch, - producer_epoch::Kind, stream_record::Item, + StreamErrorDetails, StreamHandle, StreamIdConflictPolicy, StreamReadStart, StreamRecord, + StreamStatus, StreamSuperseded, WorkflowTaskEpoch, producer_epoch::Kind, + stream_record::Item, }, }; use prost::Message; @@ -94,13 +94,8 @@ fn client(attempt: i32) -> Producer { /// A client Producer that a test names, at `attempt`. Every backend accepts it as sent. fn named(name: &str, attempt: i32) -> Producer { Producer { - kind: StreamProducerKind::Client as i32, id: named_client_producer_id(name), - epoch: Some(ProducerEpoch { - attempt, - ..Default::default() - }), - run_id: String::new(), + ..client(attempt) } } @@ -108,15 +103,15 @@ fn named(name: &str, attempt: i32) -> Producer { /// Records. The server requires the Activity's task token, so only Redis and memory accept it. fn activity(attempt: i32, scheduled: i64) -> Producer { Producer { - kind: StreamProducerKind::Activity as i32, id: activity_producer_id("r1", "a", "s"), epoch: Some(ProducerEpoch { - attempt_scheduled_time: Some(Timestamp { - seconds: scheduled, - nanos: 0, - }), - attempt, - ..Default::default() + kind: Some(Kind::Activity(ActivityEpoch { + attempt_scheduled_time: Some(Timestamp { + seconds: scheduled, + nanos: 0, + }), + attempt, + })), }), run_id: "r1".to_string(), } @@ -190,16 +185,6 @@ impl Case { call(&self.ext, "CreateStream", request).await } - async fn close(&self, producer: Producer) -> Result { - let close = CloseStreamRequest { - namespace: self.namespace.clone(), - handle: self.handle(), - producer: Some(producer), - ..Default::default() - }; - call(&self.ext, "CloseStream", close).await - } - async fn append( &self, producer: Producer, @@ -553,7 +538,7 @@ async fn an_append_with_no_sequence_takes_the_next_one_of_its_epoch() { } /// A read is inclusive from its offset, a read from the end gives only new Records, an offset past -/// the newest + 1 fails with `stream_offset`, and one below zero is invalid. +/// the newest + 1 fails with `stream_offset_too_high`, and one below zero is invalid. async fn a_read_is_inclusive_from_its_offset_and_get_offset_reads_only_new_records() { for case in cases_for(Needs::CALLS).await { case.create(None).await.unwrap(); @@ -600,7 +585,7 @@ async fn a_read_is_inclusive_from_its_offset_and_get_offset_reads_only_new_recor .unwrap_err(); assert_eq!( (error.code(), reason(&error).as_str()), - (Code::OutOfRange, "stream_offset") + (Code::OutOfRange, "stream_offset_too_high") ); let error = case .read(ReadRecordsRequest { @@ -672,7 +657,10 @@ async fn a_read_waits_for_the_create_only_as_long_as_create_wait() { tokio::time::sleep(Duration::from_millis(150)).await; let request = CreateStreamRequest { namespace, - stream_id: stream, + stream_handle: Some(StreamHandle { + id: stream, + ..Default::default() + }), ..Default::default() }; call::<_, CreateStreamResponse>(&ext, "CreateStream", request).await @@ -859,9 +847,8 @@ async fn create_owned_short(case: &Case, run: &str) { "CreateStream", CreateStreamRequest { namespace: case.namespace.clone(), - stream_id: case.stream.clone(), retention: millis(300), - owner: Some(workflow_owner(run)), + owner: Some(workflow(run)), stream_handle: owned_handle(case), ..Default::default() }, @@ -911,7 +898,7 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { let close = CloseStreamRequest { namespace: case.namespace.clone(), handle: owned_handle(&case), - producer: Some(workflow_owner(&run)), + producer: Some(workflow(&run)), ..Default::default() }; let _: CloseStreamResponse = call(&case.ext, "CloseStream", close).await.unwrap(); @@ -1013,15 +1000,6 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { } } -fn workflow_owner(run: &str) -> Producer { - Producer { - kind: StreamProducerKind::Workflow as i32, - id: crate::ids::workflow_producer_id(run), - run_id: run.to_string(), - ..Default::default() - } -} - /// The case's handle with the owner of a made-up run chain, which a create with an owner needs. fn owned_handle(case: &Case) -> Option { Some(StreamHandle { @@ -1034,15 +1012,18 @@ fn owned_handle(case: &Case) -> Option { /// 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 { - kind: StreamProducerKind::LocalActivity as i32, id: crate::ids::local_activity_producer_id(run, "call-1"), epoch: Some(ProducerEpoch { - task_started_event_id: started, - task_attempt: 1, - task_started_time: Some(Timestamp { seconds, nanos: 0 }), - attempt: 1, - ..Default::default() + kind: Some(Kind::LocalActivity(LocalActivityEpoch { + task: Some(task), + attempt: 1, + })), }), run_id: run.to_string(), } @@ -1057,8 +1038,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { "CreateStream", CreateStreamRequest { namespace: case.namespace.clone(), - stream_id: case.stream.clone(), - owner: Some(workflow_owner(&run)), + owner: Some(workflow(&run)), stream_handle: owned_handle(&case), ..Default::default() }, diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index 73974f44a..21978b692 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -14,11 +14,12 @@ use temporalio_streams::{ ids::client_producer_id, memory::MemoryStore, proto::{ - AppendRecord, AppendRecordsRequest, AppendRecordsResponse, CreateStreamRequest, - CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, - StreamErrorDetails, StreamErrorReason, StreamHandle, StreamProducerKind, + AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, + CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, + ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamHandle, + producer_epoch::Kind, }, - store::{Appended, Batch, Capabilities, Created, Entry, Owner, Store, StreamState, Withdrawal}, + store::{Appended, Batch, Capabilities, Create, Created, Entry, Store, StreamState}, }; use tokio::time::Instant; @@ -43,13 +44,9 @@ impl Store for Trimming { &self, namespace: &str, handle: &StreamHandle, - retention: Option, - owner: Owner, - withdrawal: &Withdrawal<'_>, + create: &Create<'_>, ) -> StreamResult { - self.inner - .create(namespace, handle, retention, owner, withdrawal) - .await + self.inner.create(namespace, handle, create).await } async fn append( @@ -122,7 +119,7 @@ async fn create(ext: &StreamRuntime) { "CreateStream", CreateStreamRequest { namespace: NAMESPACE.to_string(), - stream_id: STREAM.to_string(), + stream_handle: handle(), ..Default::default() }, ) @@ -131,11 +128,9 @@ async fn create(ext: &StreamRuntime) { fn client() -> Producer { Producer { - kind: StreamProducerKind::Client as i32, id: client_producer_id("me", "p"), epoch: Some(ProducerEpoch { - attempt: 1, - ..Default::default() + kind: Some(Kind::Client(ClientEpoch { attempt: 1 })), }), run_id: String::new(), } From 70541b5d09c5c7027b977308da21f53c2ea0d730 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Wed, 4 Nov 2026 22:41:42 -0700 Subject: [PATCH 06/11] Ended a read of a closed Stream with done, not a close Record. A read is done when it gives the last item of a closed Stream, and a read with no item left answers at once. A stored item that doesn't decode is corrupt Store data. --- crates/streams/src/service.rs | 72 +++++------ crates/streams/src/testing/service.rs | 174 ++++++++++++++------------ 2 files changed, 122 insertions(+), 124 deletions(-) diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index aab368b6a..34800dfbc 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -329,7 +329,8 @@ impl StreamService { }) } - /// A long poll that returns the Records it found, the close Record, or nothing after `wait`. + /// A long poll that returns the Records it found, or nothing after `wait`. On a closed Stream + /// with no item left, it returns `done` at once. pub(crate) async fn read_records( &self, request: ReadRecordsRequest, @@ -413,9 +414,9 @@ impl StreamService { } } - /// The newest `last` Records, oldest first, and the close Record of a closed Stream at the end. - /// 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. 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. async fn read_last( &self, request: ReadRecordsRequest, @@ -438,7 +439,7 @@ impl StreamService { } check_expiry(&state, 0)?; let retained = state.retained_from(); - let (mut picked, mut close) = (vec![], None); + 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; @@ -468,10 +469,7 @@ impl StreamService { if let Some(record) = entry.record { let record = decode(&record, entry.offset)?; let size = record.encoded_len(); - // The close Record ends the page and doesn't count toward the byte cap. - if matches!(record.item, Some(Item::Close(_))) { - close = Some(record); - } else if !withdrawals.iter().any(|s| withdraws(s, &record)) { + 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 { @@ -487,17 +485,17 @@ impl StreamService { if let Some(superseded) = entry.superseded { withdrawals.push( StreamSuperseded::decode(superseded.as_slice()) - .map_err(|error| StreamError::storage(error.to_string()))?, + .map_err(|error| StreamError::storage_corrupt(error.to_string()))?, ); } } } picked.reverse(); - picked.extend(close); + // The read ends at the newest offset, which a close makes final. Ok(ReadRecordsResponse { records: picked, next_offset: state.newest + 1, - done: state.closed_at.is_some(), + done: state.closed, ..Default::default() }) } @@ -525,8 +523,9 @@ impl StreamService { } } - /// The Records from `from`, or `None` when there is none yet. A page that ends in the close - /// Record is done. `(max, max_bytes)` caps the items and their bytes. + /// The Records from `from`, or `None` when there is none yet and the Stream is open. A page + /// of a closed Stream that gives its last item is done, also with no item. `(max, max_bytes)` + /// caps the items and their bytes. async fn page( &self, namespace: &str, @@ -535,39 +534,25 @@ impl StreamService { from: i64, (max, max_bytes): (usize, usize), ) -> StreamResult> { - // A read past the close returns the close Record again, so that every read ends with it. - if let Some(closed_at) = state.closed_at.filter(|at| from > *at) { - let (_, entries) = self.store.read(namespace, id, closed_at, 1).await?; - let mut records = vec![]; - for entry in entries { - if let Some(bytes) = entry.record { - records.push(decode(&bytes, entry.offset)?); - } - } - let page = ReadRecordsResponse { - records, - next_offset: from, - done: true, - ..Default::default() - }; - return Ok(Some(page)); - } // A read from the start skips the Records that the Retention hides. A read from an offset // fails on them, since the Reader asked for those Records. let retained = state.retained_from(); let mut start = if from > 0 { from } else { retained }; let mut records = vec![]; let mut next = from; - let (mut done, mut full) = (false, false); + let mut full = false; let mut bytes = 0; // The bytes and the entries that the page read from the Store. let mut pulled = (0, 0); - while records.len() < max && !done && !full { + // Whether the Stream was closed, and its newest offset, at the newest Store read. + let mut end = (state.closed, state.newest); + 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); let (chunk_state, entries) = self.store.read(namespace, id, start, chunk).await?; // 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); 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 { @@ -587,16 +572,14 @@ impl StreamService { let mut items = vec![]; if let Some(bytes) = entry.superseded { let superseded = StreamSuperseded::decode(bytes.as_slice()) - .map_err(|error| StreamError::storage(error.to_string()))?; + .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 { - let record = decode(&bytes, entry.offset)?; - done = matches!(record.item, Some(Item::Close(_))); - items.push(record); + items.push(decode(&bytes, 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. @@ -604,7 +587,7 @@ impl StreamService { pulled = (pulled.0 + size, pulled.1 + 1); let over = bytes + size > max_bytes || records.len() + items.len() > max; if !records.is_empty() && over { - (done, full) = (false, true); + full = true; next = entry.offset; break; } @@ -612,12 +595,14 @@ impl StreamService { records.extend(items); next = entry.offset + 1; start = next; - if done || records.len() >= max { + if records.len() >= max { break; } } } - if records.is_empty() { + // A closed Stream takes no more appends, so no item is left past its newest offset. + let done = end.0 && start > end.1; + if records.is_empty() && !done { return Ok(None); } let page = ReadRecordsResponse { @@ -644,8 +629,9 @@ fn chunk_len(left: usize, room: Option, (bytes, entries): (usize, usize)) } fn decode(bytes: &[u8], offset: i64) -> StreamResult { - let mut record = StreamRecord::decode(bytes) - .map_err(|error| StreamError::storage(format!("A stored Record is bad: {error}")))?; + let mut record = StreamRecord::decode(bytes).map_err(|error| { + StreamError::storage_corrupt(format!("A stored Record is bad: {error}")) + })?; record.offset = offset; Ok(record) } @@ -654,7 +640,7 @@ fn decode(bytes: &[u8], offset: i64) -> StreamResult { /// the gap. A read from the start of the Stream has no gap. /// /// A Tombstone fails every read. An open Stream that expired gets no more Records, so the Reader -/// would wait forever. A closed one lost its log and its close Record. +/// would wait forever. A closed one lost its log. fn check_expiry(state: &StreamState, from: i64) -> StreamResult<()> { if state.expired() || (from > 0 && state.trimmed >= from) { return Err(expired()); diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 8bebb898e..7933e0e07 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -12,10 +12,9 @@ use crate::{ ActivityEpoch, AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, Producer, ProducerEpoch, - ReadRecordsRequest, ReadRecordsResponse, StreamClose, StreamCloseStatus, - StreamErrorDetails, StreamHandle, StreamIdConflictPolicy, StreamReadStart, StreamRecord, - StreamStatus, StreamSuperseded, WorkflowTaskEpoch, producer_epoch::Kind, - stream_record::Item, + ReadRecordsRequest, ReadRecordsResponse, StreamErrorDetails, StreamHandle, + StreamIdConflictPolicy, StreamReadStart, StreamRecord, StreamStatus, StreamSuperseded, + WorkflowTaskEpoch, producer_epoch::Kind, stream_record::Item, }, }; use prost::Message; @@ -24,10 +23,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; -use temporalio_common::protos::temporal::api::{ - common::v1::Payload, - failure::v1::{Failure, failure::FailureInfo}, -}; +use temporalio_common::protos::temporal::api::common::v1::Payload; use tonic::Code; struct Case { @@ -239,10 +235,6 @@ impl Case { namespace: self.namespace.clone(), handle: self.handle(), producer: Some(producer), - result: Some(Payload { - data: b"done".to_vec(), - ..Default::default() - }), ..Default::default() }, ) @@ -257,7 +249,6 @@ fn texts(response: &ReadRecordsResponse) -> Vec { .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() @@ -270,13 +261,6 @@ fn superseded_of(record: &StreamRecord) -> &StreamSuperseded { } } -fn close_of(record: &StreamRecord) -> &StreamClose { - match &record.item { - Some(Item::Close(close)) => close, - other => panic!("not the close: {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() { @@ -751,19 +735,16 @@ async fn a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append( } } -/// A close ends every read with its close Record and refuses later appends. -async fn a_close_ends_every_read_with_its_record() { +/// A read that gives the last item of a closed Stream is done, and so is every later read, which +/// gives no item. A close refuses later appends. +async fn a_close_ends_every_read_with_done() { for case in cases_for(Needs::CALLS).await { case.create(None).await.unwrap(); 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); - assert_eq!( - close_of(&read.records[1]).result.as_ref().unwrap().data, - b"done" - ); let again = case .read(ReadRecordsRequest { from_offset: read.next_offset, @@ -771,8 +752,8 @@ async fn a_close_ends_every_read_with_its_record() { }) .await .unwrap(); - assert_eq!(texts(&again), [""]); - assert!(again.done); + assert!(again.records.is_empty() && again.done); + assert_eq!(again.next_offset, read.next_offset); assert_eq!( case.describe().await.unwrap().status, StreamStatus::Closed as i32 @@ -908,8 +889,8 @@ async fn a_read_behind_an_expired_log_fails_before_and_after_the_next_write() { } } -/// A close as the first write after an idle open Stream's log expired keeps the gap, so a read from -/// an offset that the log took fails as expired. +/// 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); @@ -1115,60 +1096,90 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { } } -/// A close Record takes its status from the failure. A failure is canceled when it is a cancel, or -/// an Activity, child Workflow or Nexus Operation failure whose cause is a cancel, and failed -/// otherwise. A close Record over the Record limit is refused. -async fn a_close_record_takes_its_status_from_the_failure() { - let canceled_activity = Failure { - message: "boom".to_string(), - failure_info: Some(FailureInfo::ActivityFailureInfo(Default::default())), - cause: Some(Box::new(Failure { - failure_info: Some(FailureInfo::CanceledFailureInfo(Default::default())), - ..Default::default() - })), - ..Default::default() - }; - let other = Failure { - message: "boom".to_string(), - ..Default::default() - }; - for (failure, status) in [ - (other, StreamCloseStatus::Failed), - (canceled_activity, StreamCloseStatus::Canceled), - ] { - for case in cases_for(Needs::CALLS).await { - case.create(None).await.unwrap(); - let close = CloseStreamRequest { - namespace: case.namespace.clone(), - handle: case.handle(), - producer: Some(client(1)), - failure: Some(failure.clone()), +/// 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() { + for case in cases_for(Needs::CALLS).await { + case.create(None).await.unwrap(); + let records = vec![record("1"), record("2")]; + let written = case.append(client(1), 1, records).await.unwrap(); + case.close(client(1)).await.unwrap(); + let first = case + .read(ReadRecordsRequest { + max_records: 1, ..Default::default() - }; - let huge = CloseStreamRequest { - result: Some(Payload { - data: vec![0; crate::store::MAX_RECORD_BYTES], - ..Default::default() - }), - ..close.clone() - }; - let error = call::<_, CloseStreamResponse>(&case.ext, "CloseStream", huge) - .await - .unwrap_err(); - assert_eq!(reason(&error), "stream_too_large"); - call::<_, CloseStreamResponse>(&case.ext, "CloseStream", close) + }) + .await + .unwrap(); + assert_eq!(texts(&first), ["1"]); + assert!(!first.done); + let rest = case + .read(ReadRecordsRequest { + from_offset: first.next_offset, + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(texts(&rest), ["2"]); + assert!(rest.done); + for request in [ + ReadRecordsRequest { + from_offset: written.last_offset + 1, + ..Default::default() + }, + ReadRecordsRequest { + start: StreamReadStart::End as i32, + ..Default::default() + }, + ] { + let started = Instant::now(); + let read = case + .read(ReadRecordsRequest { + wait: millis(5000), + ..request + }) .await .unwrap(); - let read = case.read(ReadRecordsRequest::default()).await.unwrap(); - let record = read.records.last().unwrap(); - assert_eq!(close_of(record).status, status as i32); - assert_eq!(close_of(record).failure.as_ref().unwrap().message, "boom"); + assert!(started.elapsed() < Duration::from_secs(2)); + assert!(read.records.is_empty() && read.done); + assert_eq!(read.next_offset, written.last_offset + 1); } } } -/// A last read gives the newest Records oldest first, skips withdrawn ones, and ends with the -/// close Record of a closed Stream. +/// A long poll at the end of an open Stream wakes on its close and answers with `done`. +async fn a_waiting_read_wakes_with_done_on_a_close() { + 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(); + let ext = case.ext.clone(); + let close = CloseStreamRequest { + namespace: case.namespace.clone(), + handle: case.handle(), + producer: Some(client(1)), + ..Default::default() + }; + let closer = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(150)).await; + call::<_, CloseStreamResponse>(&ext, "CloseStream", close).await + }); + let started = Instant::now(); + let woke = case + .read(ReadRecordsRequest { + from_offset: written.last_offset + 1, + wait: millis(5000), + ..Default::default() + }) + .await + .unwrap(); + assert!(started.elapsed() < Duration::from_secs(4)); + assert!(woke.records.is_empty() && woke.done); + closer.await.unwrap().unwrap(); + } +} + +/// 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() { for case in cases_for(Needs::CALLS).await { case.create(None).await.unwrap(); @@ -1211,7 +1222,7 @@ async fn a_last_read_gives_the_newest_records_oldest_first_without_withdrawn_one assert_eq!(reason(&error), "stream_invalid"); case.close(client(1)).await.unwrap(); let read = case.read(last(1)).await.unwrap(); - assert_eq!(texts(&read), ["a2", ""]); + assert_eq!(texts(&read), ["a2"]); assert!(read.done); } } @@ -1252,7 +1263,7 @@ pub const CASES: &[super::Case] = &[ 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_its_record), + case!(a_close_ends_every_read_with_done), 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), @@ -1261,7 +1272,8 @@ pub const CASES: &[super::Case] = &[ case!(a_handle_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_rerun_local_activity_supersedes_its_first_attempt), - case!(a_close_record_takes_its_status_from_the_failure), + 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_page_keeps_under_max_bytes_and_a_larger_record_comes_alone), ]; From 6ef54d9e8dc079d4d879284d90ac76c330f327f8 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Wed, 4 Nov 2026 22:49:25 -0700 Subject: [PATCH 07/11] Called the reads' and their suite's Stream a ref. A read and a describe carry their StreamRef in ref, as the other requests do. --- crates/streams/src/service.rs | 4 +- crates/streams/src/testing/service.rs | 64 +++++++++++++-------------- crates/streams/tests/scans.rs | 22 ++++----- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index f3d987cce..f85a1c5cb 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -337,7 +337,7 @@ impl StreamService { &self, request: ReadRecordsRequest, ) -> StreamResult { - let id = stream_id(request.handle.as_ref())?.to_string(); + let id = stream_id(request.r#ref.as_ref())?.to_string(); let namespace = request.namespace.as_str(); if request.from_offset < 0 { return Err(StreamError::invalid("The offset is below zero")); @@ -424,7 +424,7 @@ impl StreamService { request: ReadRecordsRequest, max_bytes: usize, ) -> StreamResult { - let id = stream_id(request.handle.as_ref())?.to_string(); + let id = stream_id(request.r#ref.as_ref())?.to_string(); let namespace = request.namespace.as_str(); let count = (request.last as usize).min(MAX_RECORDS); let create_wait = duration(request.create_wait) diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index a70c53d25..79b4cb263 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -13,8 +13,8 @@ use crate::{ CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, Producer, ProducerEpoch, ReadRecordsRequest, ReadRecordsResponse, StreamErrorDetails, StreamIdConflictPolicy, - StreamReadStart, StreamRecord, StreamRef, StreamStatus, StreamSuperseded, WorkflowTaskEpoch, - producer_epoch::Kind, stream_record::Item, + StreamReadStart, StreamRecord, StreamRef, StreamStatus, StreamSuperseded, + WorkflowTaskEpoch, producer_epoch::Kind, stream_record::Item, }, }; use prost::Message; @@ -208,7 +208,7 @@ impl Case { "ReadRecords", ReadRecordsRequest { namespace: self.namespace.clone(), - handle: self.handle(), + r#ref: self.stream_ref(), ..request }, ) @@ -221,7 +221,7 @@ impl Case { "DescribeStream", DescribeStreamRequest { namespace: self.namespace.clone(), - handle: self.handle(), + r#ref: self.stream_ref(), }, ) .await @@ -233,7 +233,7 @@ impl Case { "CloseStream", CloseStreamRequest { namespace: self.namespace.clone(), - handle: self.handle(), + r#ref: self.stream_ref(), producer: Some(producer), ..Default::default() }, @@ -665,7 +665,7 @@ async fn a_read_waits_for_the_create_only_as_long_as_create_wait() { tokio::time::sleep(Duration::from_millis(150)).await; let request = CreateStreamRequest { namespace, - stream_handle: Some(StreamHandle { + stream_ref: Some(StreamRef { id: stream, ..Default::default() }), @@ -711,7 +711,7 @@ async fn a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append( tokio::time::sleep(Duration::from_millis(150)).await; let request = AppendRecordsRequest { namespace, - handle: Some(StreamHandle { + r#ref: Some(StreamRef { id: stream, ..Default::default() }), @@ -854,7 +854,7 @@ async fn create_owned_short(case: &Case, run: &str) { namespace: case.namespace.clone(), retention: millis(300), owner: Some(workflow(run)), - stream_handle: owned_handle(case), + stream_ref: owned_ref(case), ..Default::default() }, ) @@ -902,7 +902,7 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { tokio::time::sleep(Duration::from_millis(400)).await; let close = CloseStreamRequest { namespace: case.namespace.clone(), - handle: owned_handle(&case), + r#ref: owned_ref(&case), producer: Some(workflow(&run)), ..Default::default() }; @@ -916,25 +916,25 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { } } -/// A handle with an owner and one without reach the same Stream. -async fn a_handle_with_an_owner_and_one_without_reach_the_same_stream() { +/// 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(StreamHandle { + 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(StreamHandle { + 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.handle()), (3, other)] { + for (sequence, stream) in [(1, owned.clone()), (2, case.stream_ref()), (3, other)] { let request = AppendRecordsRequest { namespace: case.namespace.clone(), - handle: stream, + r#ref: stream, producer: Some(client(1)), sequence, records: vec![record(&sequence.to_string())], @@ -942,12 +942,12 @@ async fn a_handle_with_an_owner_and_one_without_reach_the_same_stream() { }; let _: AppendRecordsResponse = call(&case.ext, "AppendRecords", request).await.unwrap(); } - let read = |handle: Option| ReadRecordsRequest { + let read = |stream_ref: Option| ReadRecordsRequest { namespace: case.namespace.clone(), - handle, + r#ref: stream_ref, ..Default::default() }; - let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.handle())) + let plain: ReadRecordsResponse = call(&case.ext, "ReadRecords", read(case.stream_ref())) .await .unwrap(); let with_owner: ReadRecordsResponse = @@ -968,12 +968,12 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { gives: Needs::ALL, }; let created = case.create(None).await.unwrap(); - let stream = created.handle.unwrap(); + let stream = created.r#ref.unwrap(); assert_eq!(stream.id, case.stream); assert_eq!(stream.backend, "redis-a"); - // A handle with any backend, or none, reaches the same Stream. + // A ref with any backend, or none, reaches the same Stream. let named = |backend: &str| { - Some(StreamHandle { + Some(StreamRef { id: case.stream.clone(), backend: backend.to_string(), ..Default::default() @@ -982,7 +982,7 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { for (sequence, stream) in [(1, named("redis-a")), (2, named("")), (3, named("other"))] { let request = AppendRecordsRequest { namespace: case.namespace.clone(), - handle: stream, + r#ref: stream, producer: Some(client(1)), sequence, records: vec![record(&sequence.to_string())], @@ -992,22 +992,22 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { } let read = ReadRecordsRequest { namespace: case.namespace.clone(), - handle: named("other"), + r#ref: named("other"), ..Default::default() }; let read: ReadRecordsResponse = call(&case.ext, "ReadRecords", read).await.unwrap(); assert_eq!(texts(&read), ["1", "2", "3"]); } - // With no backend in the config, the handle has none, and lang reads it as the default. + // With no backend in the config, the ref has none, and lang reads it as the default. for case in cases_for(Needs::OWN_CONFIG).await { let created = case.create(None).await.unwrap(); - assert_eq!(created.handle.unwrap().backend, ""); + assert_eq!(created.r#ref.unwrap().backend, ""); } } -/// The case's handle with the owner of a made-up run chain, which a create with an owner needs. -fn owned_handle(case: &Case) -> Option { - Some(StreamHandle { +/// 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() @@ -1044,7 +1044,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { CreateStreamRequest { namespace: case.namespace.clone(), owner: Some(workflow(&run)), - stream_handle: owned_handle(&case), + stream_ref: owned_ref(&case), ..Default::default() }, ) @@ -1052,7 +1052,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { .unwrap(); let write = |producer: Producer, text: &str| AppendRecordsRequest { namespace: case.namespace.clone(), - handle: case.handle(), + r#ref: case.stream_ref(), producer: Some(producer), sequence: 1, records: vec![record(text)], @@ -1155,7 +1155,7 @@ async fn a_waiting_read_wakes_with_done_on_a_close() { let ext = case.ext.clone(); let close = CloseStreamRequest { namespace: case.namespace.clone(), - handle: case.handle(), + r#ref: case.stream_ref(), producer: Some(client(1)), ..Default::default() }; @@ -1269,7 +1269,7 @@ pub const CASES: &[super::Case] = &[ 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_handle_with_an_owner_and_one_without_reach_the_same_stream), + 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_rerun_local_activity_supersedes_its_first_attempt), case!(a_read_of_a_closed_stream_with_no_item_left_answers_at_once), diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index 21978b692..a18dd14fc 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -16,7 +16,7 @@ use temporalio_streams::{ proto::{ AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, - ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamHandle, + ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamRef, producer_epoch::Kind, }, store::{Appended, Batch, Capabilities, Create, Created, Entry, Store, StreamState}, @@ -43,19 +43,19 @@ impl Store for Trimming { async fn create( &self, namespace: &str, - handle: &StreamHandle, + stream_ref: &StreamRef, create: &Create<'_>, ) -> StreamResult { - self.inner.create(namespace, handle, create).await + self.inner.create(namespace, stream_ref, create).await } async fn append( &self, namespace: &str, - handle: &StreamHandle, + stream_ref: &StreamRef, batch: &Batch<'_>, ) -> StreamResult { - self.inner.append(namespace, handle, batch).await + self.inner.append(namespace, stream_ref, batch).await } async fn read( @@ -119,7 +119,7 @@ async fn create(ext: &StreamRuntime) { "CreateStream", CreateStreamRequest { namespace: NAMESPACE.to_string(), - stream_handle: handle(), + stream_ref: stream_ref(), ..Default::default() }, ) @@ -136,8 +136,8 @@ fn client() -> Producer { } } -fn handle() -> Option { - Some(StreamHandle { +fn stream_ref() -> Option { + Some(StreamRef { id: STREAM.to_string(), ..Default::default() }) @@ -167,7 +167,7 @@ async fn append_sized( "AppendRecords", AppendRecordsRequest { namespace: NAMESPACE.to_string(), - handle: handle(), + r#ref: stream_ref(), producer: Some(client()), sequence, records, @@ -186,7 +186,7 @@ async fn a_trim_under_a_read_from_an_offset_fails_it_as_expired() { let first = append(&ext, 1, 3).await; let read = |from_offset| ReadRecordsRequest { namespace: NAMESPACE.to_string(), - handle: handle(), + r#ref: stream_ref(), from_offset, ..Default::default() }; @@ -214,7 +214,7 @@ async fn a_capped_read_pulls_about_its_page_from_the_store() { "ReadRecords", ReadRecordsRequest { namespace: NAMESPACE.to_string(), - handle: handle(), + r#ref: stream_ref(), max_bytes: 2500, ..Default::default() }, From cae17459f86739cb4906c7c5516891738572f9a7 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Thu, 5 Nov 2026 10:45:02 -0700 Subject: [PATCH 08/11] Sent a StreamWriter in the read suite's appends and closes. --- crates/streams/src/testing/service.rs | 17 +++++++++-------- crates/streams/tests/scans.rs | 7 +++++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 488a39f60..9678705d6 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -242,7 +242,7 @@ impl Case { CloseStreamRequest { namespace: self.namespace.clone(), r#ref: self.stream_ref(), - producer: Some(producer), + writer: writer_for(producer), ..Default::default() }, ) @@ -632,7 +632,8 @@ async fn a_higher_attempt_puts_its_superseded_item_before_its_batch() { let superseded = &all.records[1]; assert!(all.records[0].offset < superseded.offset); assert!(superseded.offset <= all.records[2].offset); - assert_eq!(superseded_of(superseded).producer_id, activity(2, 20).id); + 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 @@ -723,7 +724,7 @@ async fn a_quiet_read_ends_empty_after_its_wait_and_a_long_poll_wakes_on_append( id: stream, ..Default::default() }), - producer: Some(client(1)), + writer: writer_for(client(1)), sequence: 2, records: vec![record("2")], ..Default::default() @@ -911,7 +912,7 @@ async fn a_close_after_the_log_expired_keeps_the_gap() { let close = CloseStreamRequest { namespace: case.namespace.clone(), r#ref: owned_ref(&case), - producer: Some(workflow(&run)), + writer: writer_for(workflow(&run)), ..Default::default() }; let _: CloseStreamResponse = call(&case.ext, "CloseStream", close).await.unwrap(); @@ -943,7 +944,7 @@ async fn a_ref_with_an_owner_and_one_without_reach_the_same_stream() { let request = AppendRecordsRequest { namespace: case.namespace.clone(), r#ref: stream, - producer: Some(client(1)), + writer: writer_for(client(1)), sequence, records: vec![record(&sequence.to_string())], ..Default::default() @@ -991,7 +992,7 @@ async fn a_create_answer_names_the_backend_and_the_store_never_reads_it() { let request = AppendRecordsRequest { namespace: case.namespace.clone(), r#ref: stream, - producer: Some(client(1)), + writer: writer_for(client(1)), sequence, records: vec![record(&sequence.to_string())], ..Default::default() @@ -1061,7 +1062,7 @@ async fn a_rerun_local_activity_supersedes_its_first_attempt() { let write = |producer: Producer, text: &str| AppendRecordsRequest { namespace: case.namespace.clone(), r#ref: case.stream_ref(), - producer: Some(producer), + writer: writer_for(producer), sequence: 1, records: vec![record(text)], ..Default::default() @@ -1164,7 +1165,7 @@ async fn a_waiting_read_wakes_with_done_on_a_close() { let close = CloseStreamRequest { namespace: case.namespace.clone(), r#ref: case.stream_ref(), - producer: Some(client(1)), + writer: writer_for(client(1)), ..Default::default() }; let closer = tokio::spawn(async move { diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index a18dd14fc..bcc5ea044 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -16,7 +16,7 @@ use temporalio_streams::{ proto::{ AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, - ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamRef, + ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamRef, StreamWriter, producer_epoch::Kind, }, store::{Appended, Batch, Capabilities, Create, Created, Entry, Store, StreamState}, @@ -168,7 +168,10 @@ async fn append_sized( AppendRecordsRequest { namespace: NAMESPACE.to_string(), r#ref: stream_ref(), - producer: Some(client()), + writer: Some(StreamWriter { + producer: Some(client()), + ..Default::default() + }), sequence, records, ..Default::default() From a266cd36d30d1dea78b13ac5581621e7a6e69db2 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Thu, 5 Nov 2026 15:37:03 -0700 Subject: [PATCH 09/11] Checked that a bad read request is invalid in the runtime test. --- crates/streams/src/runtime.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/streams/src/runtime.rs b/crates/streams/src/runtime.rs index 8e495ddec..6c54a461d 100644 --- a/crates/streams/src/runtime.rs +++ b/crates/streams/src/runtime.rs @@ -200,10 +200,11 @@ pub(crate) fn decode(input: &[u8]) -> StreamResult { mod tests { use super::*; - const METHODS: [&str; 4] = [ + const METHODS: [&str; 5] = [ "CreateStream", "AppendRecords", "CloseStream", + "ReadRecords", "DescribeStream", ]; From f18356469f01dd0558dac0e1aee9e903643c2306 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Fri, 6 Nov 2026 17:13:44 -0700 Subject: [PATCH 10/11] Put the Producer kind and the writer's withdrawal into variants. The review asked to stop representing invalid states. The Producer kind is now a oneof with run_id only in the run-based variants, so a client can't carry a run. A write's task facts and withdraw_run move into RunWithdrawal, which only a run writer sets. --- .../temporal/api/stream/v1/message.proto | 102 +++++++++++------- .../streamservice/v1/request_response.proto | 2 +- .../temporal/sdk/core/streams/streams.proto | 7 +- 3 files changed, 68 insertions(+), 43 deletions(-) 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 63bca5f6c..7385028a1 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 @@ -29,27 +29,6 @@ message StreamRef { string backend = 3; } -// Experimental. Might change in the future. -// The ordered attempt of a Producer. A higher epoch supersedes the Records of a lower epoch of the -// same Producer. The case is the kind of the Producer. It decides how the Store orders the epochs, -// and which earlier Records a higher epoch withdraws. -message ProducerEpoch { - oneof kind { - // A Workflow run, which writes in its Workflow Task. A higher epoch withdraws the Records - // of the run's Workflow Tasks that never completed, also those of their Local Activities. - WorkflowTaskEpoch workflow = 1; - // A Local Activity. A higher epoch withdraws the Producer's lower epochs. - LocalActivityEpoch local_activity = 2; - // An Activity of a Workflow. A higher epoch withdraws the Producer's lower epochs. - ActivityEpoch activity = 3; - // A Standalone Activity. A higher epoch withdraws the Producer's lower epochs. - ActivityEpoch standalone_activity = 4; - // A client. A higher epoch only fences the lower ones, since a restarted client's earlier - // Records are real messages, not the Records of a failed attempt. - ClientEpoch client = 5; - } -} - // Experimental. Might change in the future. // One Workflow Task attempt of a run. It orders by started_event_id, then attempt, then // started_time. @@ -86,32 +65,78 @@ message ClientEpoch { } // Experimental. Might change in the future. -// The code that wrote a batch. Its id stays the same across attempts, and its epoch orders them. +// A Workflow run Producer. It writes in its Workflow Task. A higher epoch withdraws 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. + string run_id = 1; + WorkflowTaskEpoch epoch = 2; +} + +// Experimental. Might change in the future. +// A Local Activity Producer. A higher epoch withdraws the Producer's lower epochs. +message LocalActivityProducer { + string run_id = 1; + LocalActivityEpoch epoch = 2; +} + +// 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. +message ActivityProducer { + string run_id = 1; + ActivityEpoch epoch = 2; +} + +// Experimental. Might change in the future. +// A client Producer. A higher epoch only fences the lower ones, since a restarted client's earlier +// Records are real messages, not the Records of a failed attempt. A client has no run. +message ClientProducer { + ClientEpoch epoch = 1; +} + +// 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 +// run, so a client can't. message Producer { // Stable across attempts, such as `wf:` for the Workflow Producer of a run. string id = 1; - // Required. Its case gives the kind of the Producer. - ProducerEpoch epoch = 2; - // The run of the Producer, so nobody parses the id. Empty for a client. - string run_id = 3; + // Required. The case is the kind of the Producer. + oneof kind { + WorkflowProducer workflow = 2; + LocalActivityProducer local_activity = 3; + ActivityProducer activity = 4; + ActivityProducer standalone_activity = 5; + ClientProducer client = 6; + } } // 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. message StreamWriter { - // Required on a write. Its epoch's case picks the withdrawal rule. + // Required on a write. Its Producer's kind picks the withdrawal rule. Producer producer = 1; - // Workflow and Local Activity only. 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 = 2; - // Workflow and Local Activity only. 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 = 3; - // Retry run only. The previous run, on the retry run's first write. Every Record whose - // Producer carries it is withdrawn. - string withdraw_run = 4; + // 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. @@ -125,8 +150,7 @@ message AppendRecord { // 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. Its Producer's epoch is empty when only - // withdraw_run applies. + // The writer of the batch that withdraws, with its withdrawal facts. StreamWriter writer = 1; } 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 cdd61456e..463f91194 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 @@ -75,7 +75,7 @@ message AppendRecordsRequest { 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 withdraw_run still applies. + // 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. diff --git a/crates/protos/protos/local/temporal/sdk/core/streams/streams.proto b/crates/protos/protos/local/temporal/sdk/core/streams/streams.proto index fcec138bd..115091078 100644 --- a/crates/protos/protos/local/temporal/sdk/core/streams/streams.proto +++ b/crates/protos/protos/local/temporal/sdk/core/streams/streams.proto @@ -33,9 +33,10 @@ message MemoryStreamConfig { message OpenWriterRequest { string namespace = 1; temporal.api.stream.v1.StreamRef ref = 2; - // Each batch sends it as AppendRecords does. For a Local Activity it has the run's task facts, - // so the writes withdraw the run's orphaned Local Activity Records. Its withdraw_run must be - // empty, since only a retry run's first append withdraws a run, and a writer sends many. + // Each batch sends it as AppendRecords does. For a Local Activity its withdrawal has the run's + // task facts, so the writes withdraw the run's orphaned Local Activity Records. The withdrawal's + // withdraw_run must be empty, since only a retry run's first append withdraws a run, and a + // writer sends many. temporal.api.stream.v1.StreamWriter writer = 3; // A write sends at most this many Records in one batch. Zero is 1000. int32 max_batch_size = 4; From 8d03c96c64917c5be162b46e6028ff9739e3a192 Mon Sep 17 00:00:00 2001 From: Moe Dashti Date: Sat, 7 Nov 2026 02:49:50 -0700 Subject: [PATCH 11/11] Read the Producer kind and the writer's withdrawal through the oneofs in reads. --- crates/streams/src/epoch.rs | 127 ++++++++++++++++---------- crates/streams/src/ids.rs | 50 ++++++---- crates/streams/src/memory.rs | 4 +- crates/streams/src/record.rs | 11 ++- crates/streams/src/service.rs | 19 ++-- crates/streams/src/testing/service.rs | 89 +++++++++--------- crates/streams/src/testing/store.rs | 46 +++++----- crates/streams/src/testing/support.rs | 23 +++-- crates/streams/tests/capabilities.rs | 23 +++-- crates/streams/tests/scans.rs | 13 ++- crates/streams/tests/vectors.rs | 97 ++++++++++++-------- 11 files changed, 289 insertions(+), 213 deletions(-) diff --git a/crates/streams/src/epoch.rs b/crates/streams/src/epoch.rs index 697d66ef6..15b1ca6a2 100644 --- a/crates/streams/src/epoch.rs +++ b/crates/streams/src/epoch.rs @@ -7,8 +7,8 @@ use crate::{ StreamError, StreamResult, proto::{ - Producer, ProducerEpoch, StreamRecord, StreamSuperseded, WorkflowTaskEpoch, - producer_epoch::Kind, stream_record::Item, + Producer, StreamRecord, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, producer::Kind, + stream_record::Item, }, }; use prost_types::Timestamp; @@ -42,22 +42,52 @@ impl From<&Kind> for ProducerKind { } } -/// Refuses a Producer with no epoch, or with an epoch that has no case. +/// Refuses a Producer with no kind case. impl TryFrom<&Producer> for ProducerKind { type Error = StreamError; fn try_from(producer: &Producer) -> StreamResult { - let epoch = producer - .epoch - .as_ref() - .ok_or_else(|| StreamError::invalid("The Producer has no epoch"))?; - kind(epoch).ok_or_else(|| StreamError::invalid("The Producer's epoch has no kind")) + kind_of(producer).ok_or_else(|| StreamError::invalid("The Producer has no kind")) } } -/// The kind of an epoch's case, `None` when the epoch has no case. -fn kind(epoch: &ProducerEpoch) -> Option { - epoch.kind.as_ref().map(ProducerKind::from) +/// The kind case of a Producer, `None` when it has none. +pub fn kind_of(producer: &Producer) -> Option { + producer.kind.as_ref().map(ProducerKind::from) +} + +/// The run of a Producer. Empty for a client, which has no run. +pub fn producer_run_id(producer: &Producer) -> &str { + match &producer.kind { + Some(Kind::Workflow(w)) => &w.run_id, + Some(Kind::LocalActivity(l)) => &l.run_id, + Some(Kind::Activity(a) | Kind::StandaloneActivity(a)) => &a.run_id, + Some(Kind::Client(_)) | None => "", + } +} + +/// 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]. @@ -92,20 +122,24 @@ pub fn task_key(task: &WorkflowTaskEpoch) -> [u8; TASK_KEY_LEN] { } /// The key of an epoch. It is the [task_of] key, then the attempt's scheduled seconds and nanos -/// and the attempt, in 8, 4 and 4 bytes. A field that the case doesn't have is zero, and an epoch +/// and the attempt, in 8, 4 and 4 bytes. A field that the case doesn't have is zero, and a Producer /// with no case is all zeros. -pub fn epoch_key(epoch: &ProducerEpoch) -> [u8; EPOCH_KEY_LEN] { - let (scheduled, attempt) = match &epoch.kind { - Some(Kind::LocalActivity(local)) => (None, local.attempt), +pub fn epoch_key(kind: Option<&Kind>) -> [u8; EPOCH_KEY_LEN] { + let (scheduled, attempt) = match kind { + Some(Kind::LocalActivity(local)) => (None, local.epoch.map_or(0, |e| e.attempt)), Some(Kind::Activity(activity) | Kind::StandaloneActivity(activity)) => { - (activity.attempt_scheduled_time.as_ref(), activity.attempt) + let epoch = activity.epoch; + ( + epoch.and_then(|e| e.attempt_scheduled_time), + epoch.map_or(0, |e| e.attempt), + ) } - Some(Kind::Client(client)) => (None, client.attempt), + Some(Kind::Client(client)) => (None, client.epoch.map_or(0, |e| e.attempt)), Some(Kind::Workflow(_)) | None => (None, 0), }; - let (seconds, nanos) = time_fields(scheduled); + let (seconds, nanos) = time_fields(scheduled.as_ref()); let mut key = [0; EPOCH_KEY_LEN]; - key[..TASK_KEY_LEN].copy_from_slice(&task_key(&task_of(epoch))); + key[..TASK_KEY_LEN].copy_from_slice(&task_key(&task_of(kind))); key[24..32].copy_from_slice(&be64(seconds)); key[32..36].copy_from_slice(&be32(nanos)); key[36..].copy_from_slice(&be32(attempt)); @@ -114,16 +148,16 @@ pub fn epoch_key(epoch: &ProducerEpoch) -> [u8; EPOCH_KEY_LEN] { /// The Workflow Task of an epoch. It is a Workflow's own task, or the task that dispatched a Local /// Activity, and zero for the other kinds. -pub fn task_of(epoch: &ProducerEpoch) -> WorkflowTaskEpoch { - match &epoch.kind { - Some(Kind::Workflow(task)) => *task, - Some(Kind::LocalActivity(local)) => local.task.unwrap_or_default(), +pub fn task_of(kind: Option<&Kind>) -> WorkflowTaskEpoch { + match kind { + Some(Kind::Workflow(w)) => w.epoch.unwrap_or_default(), + Some(Kind::LocalActivity(local)) => local.epoch.and_then(|e| e.task).unwrap_or_default(), _ => WorkflowTaskEpoch::default(), } } /// Orders two epochs field by field. -pub fn compare_epochs(a: &ProducerEpoch, b: &ProducerEpoch) -> Ordering { +pub fn compare_epochs(a: Option<&Kind>, b: Option<&Kind>) -> Ordering { epoch_key(a).cmp(&epoch_key(b)) } @@ -150,48 +184,52 @@ pub fn withdraws(superseded: &StreamSuperseded, record: &StreamRecord) -> bool { let Some(producer) = data.producer.as_ref() else { return false; }; - if !writer.withdraw_run.is_empty() && producer.run_id == writer.withdraw_run { + 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 (Some(epoch), Some(old)) = (withdrawing.epoch.as_ref(), producer.epoch.as_ref()) else { + let (new, old) = (withdrawing.kind.as_ref(), producer.kind.as_ref()); + let (Some(new_kind), Some(old_kind)) = (new, old) else { return false; }; - match kind(epoch) { - None | Some(ProducerKind::Client) => return false, - Some(ProducerKind::Workflow) => {} + match ProducerKind::from(new_kind) { + ProducerKind::Client => return false, + ProducerKind::Workflow => {} same => { - return kind(old) == same + return ProducerKind::from(old_kind) == same && producer.id == withdrawing.id - && compare_epochs(old, epoch) == Ordering::Less; + && compare_epochs(old, new) == Ordering::Less; } } if !of_run(producer, withdrawing) { return false; } - let (task, old) = (task_of(epoch), task_of(old)); - let started = old.started_event_id; - if writer.failed_tasks.contains(&started) { + 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.as_ref(); + 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, at) == Ordering::Less + 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 producer.epoch.as_ref().and_then(kind) { + match kind_of(producer) { Some(ProducerKind::Workflow) => producer.id == withdrawing.id, - Some(ProducerKind::LocalActivity) => producer.run_id == withdrawing.run_id, + Some(ProducerKind::LocalActivity) => { + producer_run_id(producer) == producer_run_id(withdrawing) + } _ => false, } } @@ -199,17 +237,14 @@ fn of_run(producer: &Producer, withdrawing: &Producer) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::proto::ClientEpoch; + use crate::proto::{ClientEpoch, ClientProducer, producer::Kind}; // Without the clamp, the bytes of a negative field would outrank every real epoch. #[test] fn a_negative_field_compares_as_zero() { - let negative = ProducerEpoch { - kind: Some(Kind::Client(ClientEpoch { attempt: -1 })), - }; - assert_eq!( - compare_epochs(&negative, &ProducerEpoch::default()), - Ordering::Equal - ); + let negative = Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt: -1 }), + }); + assert_eq!(compare_epochs(Some(&negative), None), Ordering::Equal); } } diff --git a/crates/streams/src/ids.rs b/crates/streams/src/ids.rs index d50cf47f3..45eb1e77b 100644 --- a/crates/streams/src/ids.rs +++ b/crates/streams/src/ids.rs @@ -11,7 +11,11 @@ //! script. Each part is percent-encoded, so a `:` or a brace in an id can't make two Streams share //! a key. -use crate::{StreamError, StreamResult, epoch::ProducerKind, proto::Producer}; +use crate::{ + StreamError, StreamResult, + epoch::{ProducerKind, producer_run_id}, + proto::Producer, +}; /// The longest Stream id, in bytes. pub const MAX_STREAM_ID_BYTES: usize = 4096; @@ -151,13 +155,14 @@ pub fn check_producer(producer: &Producer) -> StreamResult<()> { kind, ProducerKind::Workflow | ProducerKind::LocalActivity | ProducerKind::Activity ); - if runs && decode_part(parts[0]).as_deref() != Some(producer.run_id.as_str()) { + if runs && decode_part(parts[0]).as_deref() != Some(producer_run_id(producer)) { return Err(StreamError::invalid(format!( "The Producer id {:?} names another run than its run id {:?}", - producer.id, producer.run_id + producer.id, + producer_run_id(producer) ))); } - if kind == ProducerKind::Client && !producer.run_id.is_empty() { + if kind == ProducerKind::Client && !producer_run_id(producer).is_empty() { return Err(StreamError::invalid("A client Producer has no run id")); } Ok(()) @@ -207,20 +212,34 @@ impl StreamKeys { #[cfg(test)] mod tests { use super::*; - use crate::proto::{ProducerEpoch, producer_epoch::Kind}; + use crate::proto::{ + ActivityProducer, ClientProducer, LocalActivityProducer, WorkflowProducer, producer::Kind, + }; fn producer(kind: ProducerKind, id: &str, run_id: &str) -> Producer { + let run_id = run_id.to_string(); let kind = match kind { - ProducerKind::Workflow => Kind::Workflow(Default::default()), - ProducerKind::LocalActivity => Kind::LocalActivity(Default::default()), - ProducerKind::Activity => Kind::Activity(Default::default()), - ProducerKind::StandaloneActivity => Kind::StandaloneActivity(Default::default()), - ProducerKind::Client => Kind::Client(Default::default()), + ProducerKind::Workflow => Kind::Workflow(WorkflowProducer { + run_id, + ..Default::default() + }), + ProducerKind::LocalActivity => Kind::LocalActivity(LocalActivityProducer { + run_id, + ..Default::default() + }), + ProducerKind::Activity => Kind::Activity(ActivityProducer { + run_id, + ..Default::default() + }), + ProducerKind::StandaloneActivity => Kind::StandaloneActivity(ActivityProducer { + run_id, + ..Default::default() + }), + ProducerKind::Client => Kind::Client(ClientProducer::default()), }; Producer { id: id.to_string(), - epoch: Some(ProducerEpoch { kind: Some(kind) }), - run_id: run_id.to_string(), + kind: Some(kind), } } @@ -267,14 +286,9 @@ mod tests { producer(ProducerKind::Workflow, &workflow_producer_id(run), "other"), producer(ProducerKind::Activity, "act:run%3A1:a", run), Producer { - epoch: None, - ..producer(ProducerKind::Workflow, "wf:x", "x") - }, - Producer { - epoch: Some(ProducerEpoch::default()), + kind: None, ..producer(ProducerKind::Workflow, "wf:x", "x") }, - producer(ProducerKind::Client, "client:a:b", "run"), producer(ProducerKind::Client, "client:a|b", ""), producer(ProducerKind::Client, "client:run%3a1", ""), ]; diff --git a/crates/streams/src/memory.rs b/crates/streams/src/memory.rs index 52804d291..4cd315d1f 100644 --- a/crates/streams/src/memory.rs +++ b/crates/streams/src/memory.rs @@ -578,7 +578,7 @@ impl Store for MemoryStore { let stream_id = stream_ref.id.as_str(); let producer = batch.producer; let kind = ProducerKind::try_from(producer)?; - let epoch = epoch_key(&producer.epoch.unwrap_or_default()); + let epoch = epoch_key(producer.kind.as_ref()); let withdrawal = &batch.withdrawal; let (safety, tombstone) = (millis(self.safety_ttl), millis(TOMBSTONE)); let field = format!("p|{}|{}", kind as i32, producer.id); @@ -721,7 +721,7 @@ impl Store for MemoryStore { ) -> StreamResult { let stream_id = stream_ref.id.as_str(); let kind = ProducerKind::try_from(producer)?; - let epoch = epoch_key(&producer.epoch.unwrap_or_default()); + let epoch = epoch_key(producer.kind.as_ref()); let field = format!("p|{}|{}", kind as i32, producer.id); let tombstone = millis(TOMBSTONE); let chain = owner_chain(stream_ref); diff --git a/crates/streams/src/record.rs b/crates/streams/src/record.rs index 85d32e1f4..2277adf9b 100644 --- a/crates/streams/src/record.rs +++ b/crates/streams/src/record.rs @@ -3,8 +3,8 @@ use crate::{ Reason, StreamError, StreamResult, proto::{ - AppendRecord, Producer, StreamData, StreamRecord, StreamSuperseded, StreamWriter, - stream_record::Item, + AppendRecord, Producer, RunWithdrawal, StreamData, StreamRecord, StreamSuperseded, + StreamWriter, stream_record::Item, }, store::{MAX_BATCH_BYTES, MAX_BATCH_RECORDS, MAX_RECORD_BYTES}, }; @@ -65,11 +65,14 @@ pub(crate) fn data_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()), - withdraw_run: withdraw_run.to_string(), - ..Default::default() + withdrawal, }), } .encode_to_vec() diff --git a/crates/streams/src/service.rs b/crates/streams/src/service.rs index 7899b9992..2c811f7aa 100644 --- a/crates/streams/src/service.rs +++ b/crates/streams/src/service.rs @@ -4,7 +4,7 @@ use crate::{ Reason, StreamError, StreamResult, digest::batch_digest, - epoch::{ProducerKind, task_of, withdraws}, + epoch::{ProducerKind, producer_run_id, task_of, withdraws, writer_withdraw_run}, ids::{check_producer, check_stream_id}, metrics::Metrics, proto::{ @@ -107,7 +107,7 @@ fn stream_id(stream_ref: Option<&StreamRef>) -> StreamResult<&str> { /// so a Reader withdraws only that run's Records. fn run_superseded(owner: Option<&Producer>, withdraw_run: &str) -> Vec { let producer = Producer { - epoch: None, + kind: None, ..owner.cloned().unwrap_or_default() }; superseded(&producer, withdraw_run) @@ -124,9 +124,7 @@ fn outside_producer(producer: Option<&Producer>) -> StreamResult<&Producer> { )); } // Without its task, a re-dispatch in a later task has the same epoch as the first dispatch. - let task = producer - .epoch - .map_or(0, |epoch| task_of(&epoch).started_event_id); + let task = task_of(producer.kind.as_ref()).started_event_id; if kind == ProducerKind::LocalActivity && task <= 0 { return Err(StreamError::invalid( "A Local Activity Producer's epoch must name the Workflow Task that dispatched it", @@ -189,7 +187,7 @@ impl StreamService { "A create with withdraw_run needs the owner run", )); }; - if request.withdraw_run == owner.run_id { + 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)?; @@ -248,10 +246,11 @@ impl StreamService { .capabilities() .require(Capabilities::NEXT_SEQUENCE)?; } - if !writer.withdraw_run.is_empty() { + 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 || writer.withdraw_run == producer.run_id { + 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", )); @@ -260,14 +259,14 @@ impl StreamService { } let digest = batch_digest(&request.records); let records = data_records(producer, request.records)?; - let bytes = superseded(producer, &writer.withdraw_run); + let bytes = superseded(producer, withdraw_run); let batch = Batch { producer, sequence: request.sequence, records: &records, digest, withdrawal: Withdrawal { - withdraw_run: &writer.withdraw_run, + withdraw_run, superseded: &bytes, tasks: None, }, diff --git a/crates/streams/src/testing/service.rs b/crates/streams/src/testing/service.rs index 9678705d6..88e6245f4 100644 --- a/crates/streams/src/testing/service.rs +++ b/crates/streams/src/testing/service.rs @@ -9,12 +9,13 @@ use crate::{ epoch::withdraws, ids::{activity_producer_id, client_producer_id, named_client_producer_id}, proto::{ - ActivityEpoch, AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, - CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, CreateStreamResponse, - DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, Producer, ProducerEpoch, - ReadRecordsRequest, ReadRecordsResponse, StreamErrorDetails, StreamIdConflictPolicy, - StreamReadStart, StreamRecord, StreamRef, StreamStatus, StreamSuperseded, StreamWriter, - WorkflowTaskEpoch, producer_epoch::Kind, stream_record::Item, + ActivityEpoch, ActivityProducer, AppendRecord, AppendRecordsRequest, AppendRecordsResponse, + ClientEpoch, ClientProducer, CloseStreamRequest, CloseStreamResponse, CreateStreamRequest, + CreateStreamResponse, DescribeStreamRequest, DescribeStreamResponse, LocalActivityEpoch, + LocalActivityProducer, Producer, ReadRecordsRequest, ReadRecordsResponse, + StreamErrorDetails, StreamIdConflictPolicy, StreamReadStart, StreamRecord, StreamRef, + StreamStatus, StreamSuperseded, StreamWriter, WorkflowProducer, WorkflowTaskEpoch, + producer::Kind, stream_record::Item, }, }; use prost::Message; @@ -88,10 +89,9 @@ fn writer_for(producer: Producer) -> Option { fn client(attempt: i32) -> Producer { Producer { id: client_producer_id("me", "p"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Client(ClientEpoch { attempt })), - }), - run_id: String::new(), + kind: Some(Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt }), + })), } } @@ -108,16 +108,16 @@ fn named(name: &str, attempt: i32) -> Producer { fn activity(attempt: i32, scheduled: i64) -> Producer { Producer { id: activity_producer_id("r1", "a", "s"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Activity(ActivityEpoch { + kind: Some(Kind::Activity(ActivityProducer { + run_id: "r1".to_string(), + epoch: Some(ActivityEpoch { attempt_scheduled_time: Some(Timestamp { seconds: scheduled, nanos: 0, }), attempt, - })), - }), - run_id: "r1".to_string(), + }), + })), } } @@ -125,10 +125,10 @@ fn activity(attempt: i32, scheduled: i64) -> Producer { fn workflow(run: &str) -> Producer { Producer { id: crate::ids::workflow_producer_id(run), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Workflow(WorkflowTaskEpoch::default())), - }), - run_id: run.to_string(), + kind: Some(Kind::Workflow(WorkflowProducer { + run_id: run.to_string(), + epoch: Some(WorkflowTaskEpoch::default()), + })), } } @@ -388,13 +388,13 @@ async fn a_workflow_or_a_client_closes_a_stream_with_no_owner() { let run = format!("{}-run", case.stream); let activity = Producer { id: activity_producer_id(&run, "a", "s"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Activity(ActivityEpoch { + kind: Some(Kind::Activity(ActivityProducer { + run_id: run.clone(), + epoch: Some(ActivityEpoch { attempt: 1, ..Default::default() - })), - }), - run_id: run.clone(), + }), + })), }; let error = case.close(activity).await.unwrap_err(); assert_eq!( @@ -432,13 +432,13 @@ async fn a_create_checks_its_owner_and_its_ref() { }; let local_activity = Producer { id: crate::ids::local_activity_producer_id(&run, "call-1"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::LocalActivity(LocalActivityEpoch { + kind: Some(Kind::LocalActivity(LocalActivityProducer { + run_id: run.clone(), + epoch: Some(LocalActivityEpoch { task: Some(task), attempt: 1, - })), - }), - ..workflow.clone() + }), + })), }; let chain = |workflow_id: &str| StreamRef { id: case.stream.clone(), @@ -500,11 +500,12 @@ async fn a_create_checks_its_owner_and_its_ref() { async fn a_producer_must_match_its_kind_and_not_be_a_workflow() { for case in cases_for(Needs::CALLS).await { case.create(None).await.unwrap(); - let activity = ProducerEpoch { - kind: Some(Kind::Activity(ActivityEpoch::default())), - }; - for epoch in [Some(activity), None, Some(ProducerEpoch::default())] { - let wrong = Producer { epoch, ..client(1) }; + let activity = Kind::Activity(ActivityProducer { + run_id: String::new(), + epoch: Some(ActivityEpoch::default()), + }); + for kind in [Some(activity), None] { + let wrong = Producer { kind, ..client(1) }; let error = case.append(wrong, 1, vec![record("x")]).await.unwrap_err(); assert_eq!( (error.code(), reason(&error).as_str()), @@ -515,13 +516,13 @@ async fn a_producer_must_match_its_kind_and_not_be_a_workflow() { assert_eq!(error.code(), Code::InvalidArgument); let no_task = Producer { id: crate::ids::local_activity_producer_id("r1", "call-1"), - run_id: "r1".to_string(), - epoch: Some(ProducerEpoch { - kind: Some(Kind::LocalActivity(LocalActivityEpoch { + kind: Some(Kind::LocalActivity(LocalActivityProducer { + run_id: "r1".to_string(), + epoch: Some(LocalActivityEpoch { task: None, attempt: 1, - })), - }), + }), + })), }; let error = case .append(no_task, 1, vec![record("x")]) @@ -1033,13 +1034,13 @@ fn user_local_activity(run: &str, started: i64, seconds: i64) -> Producer { }; Producer { id: crate::ids::local_activity_producer_id(run, "call-1"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::LocalActivity(LocalActivityEpoch { + kind: Some(Kind::LocalActivity(LocalActivityProducer { + run_id: run.to_string(), + epoch: Some(LocalActivityEpoch { task: Some(task), attempt: 1, - })), - }), - run_id: run.to_string(), + }), + })), } } diff --git a/crates/streams/src/testing/store.rs b/crates/streams/src/testing/store.rs index 29ade82d1..dc9288ec1 100644 --- a/crates/streams/src/testing/store.rs +++ b/crates/streams/src/testing/store.rs @@ -17,8 +17,8 @@ use crate::{ memory::{EntryId, ProducerField}, offset::{entry_id_of_offset, offset_of_entry_id}, proto::{ - ActivityEpoch, ClientEpoch, Producer, ProducerEpoch, StreamRef, WorkflowTaskEpoch, - producer_epoch::Kind, + ActivityEpoch, ActivityProducer, ClientEpoch, ClientProducer, Producer, StreamRef, + WorkflowProducer, WorkflowTaskEpoch, producer::Kind, }, store::{Batch, ConflictPolicy, Create, CreateOutcome, Created, Owner, Withdrawal}, }; @@ -29,26 +29,25 @@ use std::time::Duration; fn activity(run: &str, attempt: i32, scheduled: i64) -> Producer { Producer { id: activity_producer_id(run, "a", "s"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Activity(ActivityEpoch { + kind: Some(Kind::Activity(ActivityProducer { + run_id: run.to_string(), + epoch: Some(ActivityEpoch { attempt_scheduled_time: Some(Timestamp { seconds: scheduled, nanos: 0, }), attempt, - })), - }), - run_id: run.to_string(), + }), + })), } } fn client(attempt: i32) -> Producer { Producer { id: client_producer_id("me", "p1"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Client(ClientEpoch { attempt })), - }), - run_id: String::new(), + kind: Some(Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt }), + })), } } @@ -718,10 +717,10 @@ async fn a_stale_producer_cant_close_the_stream() { fn workflow(run: &str) -> Producer { Producer { id: workflow_producer_id(run), - run_id: run.to_string(), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Workflow(WorkflowTaskEpoch::default())), - }), + kind: Some(Kind::Workflow(WorkflowProducer { + run_id: run.to_string(), + epoch: Some(WorkflowTaskEpoch::default()), + })), } } @@ -744,13 +743,14 @@ async fn a_workflow_or_a_client_closes_a_stream_with_no_owner() { async fn a_producer_with_no_epoch_case_is_refused() { for target in stores_for(Needs::CALLS).await { target.create(Owner::Client).await; - for epoch in [None, Some(ProducerEpoch::default())] { - let producer = Producer { epoch, ..client(1) }; - let error = append(&target, &producer, 1, &["a"]).await.unwrap_err(); - assert_eq!(error.reason, Reason::Invalid); - let error = target.close(&producer).await.unwrap_err(); - assert_eq!(error.reason, Reason::Invalid); - } + let producer = Producer { + kind: None, + ..client(1) + }; + let error = append(&target, &producer, 1, &["a"]).await.unwrap_err(); + assert_eq!(error.reason, Reason::Invalid); + let error = target.close(&producer).await.unwrap_err(); + assert_eq!(error.reason, Reason::Invalid); assert!(target.texts().await.is_empty()); } } @@ -856,7 +856,7 @@ async fn a_producer_field_has_the_shared_layout() { target.create(Owner::Workflow).await; let kind = ProducerKind::Activity as i32; let field = |producer: &Producer, held: Option<&Producer>, offsets, texts: &[&str]| { - let epoch = |producer: &Producer| epoch_key(&producer.epoch.unwrap_or_default()); + let epoch = |producer: &Producer| epoch_key(producer.kind.as_ref()); ProducerField { fence: epoch(producer), held: held.map(epoch), diff --git a/crates/streams/src/testing/support.rs b/crates/streams/src/testing/support.rs index a018402b9..fcaa45fec 100644 --- a/crates/streams/src/testing/support.rs +++ b/crates/streams/src/testing/support.rs @@ -10,9 +10,9 @@ use crate::{ epoch::withdraws, ids::local_activity_producer_id, proto::{ - AppendRecord, LocalActivityEpoch, Producer, ProducerEpoch, StreamData, StreamRecord, - StreamRef, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, producer_epoch::Kind, - stream_record::Item, + AppendRecord, LocalActivityEpoch, LocalActivityProducer, Producer, RunWithdrawal, + StreamData, StreamRecord, StreamRef, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, + producer::Kind, stream_record::Item, }, store::{Appended, Batch, Closed, Create, Created, Entry, Owner, Store, Withdrawal}, }; @@ -132,13 +132,13 @@ impl Run { }; Producer { id: local_activity_producer_id(&self.id, call), - epoch: Some(ProducerEpoch { - kind: Some(Kind::LocalActivity(LocalActivityEpoch { + kind: Some(Kind::LocalActivity(LocalActivityProducer { + run_id: self.id.clone(), + epoch: Some(LocalActivityEpoch { task: Some(task), attempt, - })), - }), - run_id: self.id.clone(), + }), + })), } } } @@ -185,11 +185,14 @@ pub(crate) fn digest(texts: &[&str]) -> [u8; crate::digest::DIGEST_LEN] { } 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()), - withdraw_run: withdrawal.withdraw_run.to_string(), - ..Default::default() + withdrawal: run, }), } .encode_to_vec() diff --git a/crates/streams/tests/capabilities.rs b/crates/streams/tests/capabilities.rs index 2b312f1da..34212ccc9 100644 --- a/crates/streams/tests/capabilities.rs +++ b/crates/streams/tests/capabilities.rs @@ -12,10 +12,10 @@ use temporalio_streams::{ ids::client_producer_id, memory::MemoryStore, proto::{ - AppendRecord, AppendRecordsRequest, ClientEpoch, CloseStreamRequest, CreateStreamRequest, - DescribeStreamRequest, DescribeStreamResponse, Producer, ProducerEpoch, StreamErrorDetails, - StreamErrorReason, StreamRef, StreamStatus, StreamWriter, WorkflowTaskEpoch, - producer_epoch::Kind, + AppendRecord, AppendRecordsRequest, ClientEpoch, ClientProducer, CloseStreamRequest, + CreateStreamRequest, DescribeStreamRequest, DescribeStreamResponse, Producer, + StreamErrorDetails, StreamErrorReason, StreamRef, StreamStatus, StreamWriter, + WorkflowProducer, WorkflowTaskEpoch, producer::Kind, }, store::{Appended, Batch, Capabilities, Closed, Create, Created, Entry, Store, StreamState}, }; @@ -120,10 +120,9 @@ fn case(without: Capabilities) -> Case { fn client() -> Producer { Producer { id: client_producer_id("me", "p"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Client(ClientEpoch { attempt: 1 })), - }), - run_id: String::new(), + kind: Some(Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt: 1 }), + })), } } @@ -186,10 +185,10 @@ impl Case { withdraw_run: withdraw_run.to_string(), owner: Some(Producer { id: temporalio_streams::ids::workflow_producer_id(&self.run_id), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Workflow(WorkflowTaskEpoch::default())), - }), - run_id: self.run_id.clone(), + kind: Some(Kind::Workflow(WorkflowProducer { + run_id: self.run_id.clone(), + epoch: Some(WorkflowTaskEpoch::default()), + })), }), stream_ref: Some(StreamRef { id: self.stream.clone(), diff --git a/crates/streams/tests/scans.rs b/crates/streams/tests/scans.rs index bcc5ea044..e2e5f54c4 100644 --- a/crates/streams/tests/scans.rs +++ b/crates/streams/tests/scans.rs @@ -14,10 +14,10 @@ use temporalio_streams::{ ids::client_producer_id, memory::MemoryStore, proto::{ - AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, - CreateStreamRequest, CreateStreamResponse, Producer, ProducerEpoch, ReadRecordsRequest, + AppendRecord, AppendRecordsRequest, AppendRecordsResponse, ClientEpoch, ClientProducer, + CreateStreamRequest, CreateStreamResponse, Producer, ReadRecordsRequest, ReadRecordsResponse, StreamErrorDetails, StreamErrorReason, StreamRef, StreamWriter, - producer_epoch::Kind, + producer::Kind, }, store::{Appended, Batch, Capabilities, Create, Created, Entry, Store, StreamState}, }; @@ -129,10 +129,9 @@ async fn create(ext: &StreamRuntime) { fn client() -> Producer { Producer { id: client_producer_id("me", "p"), - epoch: Some(ProducerEpoch { - kind: Some(Kind::Client(ClientEpoch { attempt: 1 })), - }), - run_id: String::new(), + kind: Some(Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt: 1 }), + })), } } diff --git a/crates/streams/tests/vectors.rs b/crates/streams/tests/vectors.rs index 853a4e96c..d824561a4 100644 --- a/crates/streams/tests/vectors.rs +++ b/crates/streams/tests/vectors.rs @@ -12,9 +12,10 @@ use temporalio_streams::{ ids::{self, StreamKeys}, offset::{entry_id_of_offset, offset_of_entry_id}, proto::{ - ActivityEpoch, AppendRecord, ClientEpoch, LocalActivityEpoch, Producer, ProducerEpoch, - StreamData, StreamRecord, StreamSuperseded, StreamWriter, WorkflowTaskEpoch, - producer_epoch::Kind, stream_record::Item, + ActivityEpoch, ActivityProducer, AppendRecord, ClientEpoch, ClientProducer, + LocalActivityEpoch, LocalActivityProducer, Producer, RunWithdrawal, StreamData, + StreamRecord, StreamSuperseded, StreamWriter, WorkflowProducer, WorkflowTaskEpoch, + producer::Kind, stream_record::Item, }, }; @@ -39,31 +40,43 @@ fn number(value: &Value, field: &str) -> i64 { value[field].as_i64().unwrap_or(0) } -/// The epoch of a kind, from the flat fields of the vectors. The case takes the fields of its kind. -fn epoch(kind: &str, value: &Value) -> ProducerEpoch { +/// The Producer kind of a vector, from its flat fields and its run. The case takes the fields of +/// its kind, and a run-based kind carries the run. `empty` has no case. +fn kind_of(kind: &str, value: &Value, run_id: &str) -> Option { let task = WorkflowTaskEpoch { started_event_id: number(value, "task_started_event_id"), attempt: number(value, "task_attempt") as i32, started_time: time(&value["task_started_time"]), }; let attempt = number(value, "attempt") as i32; - let activity = ActivityEpoch { - attempt_scheduled_time: time(&value["attempt_scheduled_time"]), - attempt, - }; - let kind = match kind { - "workflow" => Some(Kind::Workflow(task)), - "local_activity" => Some(Kind::LocalActivity(LocalActivityEpoch { - task: Some(task), + let run_id = run_id.to_string(); + let activity = ActivityProducer { + run_id: run_id.clone(), + epoch: Some(ActivityEpoch { + attempt_scheduled_time: time(&value["attempt_scheduled_time"]), attempt, + }), + }; + match kind { + "workflow" => Some(Kind::Workflow(WorkflowProducer { + run_id, + epoch: Some(task), + })), + "local_activity" => Some(Kind::LocalActivity(LocalActivityProducer { + run_id, + epoch: Some(LocalActivityEpoch { + task: Some(task), + attempt, + }), })), "activity" => Some(Kind::Activity(activity)), "standalone_activity" => Some(Kind::StandaloneActivity(activity)), - "client" => Some(Kind::Client(ClientEpoch { attempt })), + "client" => Some(Kind::Client(ClientProducer { + epoch: Some(ClientEpoch { attempt }), + })), "empty" => None, other => panic!("unknown kind {other}"), - }; - ProducerEpoch { kind } + } } #[test] @@ -137,10 +150,10 @@ fn offsets_match() { #[test] fn epoch_keys_match() { for case in load("epochs.json").as_array().unwrap() { - let epoch = epoch(text(case, "kind"), &case["epoch"]); - assert_eq!(epoch_key(&epoch).to_vec(), hex(text(case, "key"))); + let kind = kind_of(text(case, "kind"), &case["epoch"], ""); + assert_eq!(epoch_key(kind.as_ref()).to_vec(), hex(text(case, "key"))); assert_eq!( - task_key(&task_of(&epoch)).to_vec(), + task_key(&task_of(kind.as_ref())).to_vec(), hex(text(case, "task_key")) ); } @@ -152,29 +165,34 @@ fn the_appendix_e_withdrawals_match() { 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(), - // A create's item has no epoch, and a Reader must not read it as an empty one. - epoch: s - .get("epoch") - .map(|value| epoch(text(s, "producer_kind"), value)), - run_id: s["run_id"].as_str().unwrap_or_default().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), - 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"]), - } + 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(), }), - 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"]; @@ -182,8 +200,13 @@ fn the_appendix_e_withdrawals_match() { item: Some(Item::Data(StreamData { producer: Some(Producer { id: text(p, "id").to_string(), - epoch: p.get("epoch").map(|value| epoch(text(p, "kind"), value)), - run_id: p["run_id"].as_str().unwrap_or_default().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() })),