From fa9e81379c7822c1d80b14f5361670fbd336de92 Mon Sep 17 00:00:00 2001 From: Ben Kirwin Date: Wed, 15 Oct 2025 16:10:35 -0400 Subject: [PATCH] Relax assertions when unflattening h/t to Dov for spotting the issue... we don't diff thin spine batches the way we ought to, which means that even correct diffs will allow the list of batches to get out of sync. --- src/persist-client/src/internal/trace.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/persist-client/src/internal/trace.rs b/src/persist-client/src/internal/trace.rs index 2b7bfe5272e63..16ed0c5fe9f21 100644 --- a/src/persist-client/src/internal/trace.rs +++ b/src/persist-client/src/internal/trace.rs @@ -66,6 +66,7 @@ use serde::{Serialize, Serializer}; use timely::PartialOrder; use timely::progress::frontier::AntichainRef; use timely::progress::{Antichain, Timestamp}; +use tracing::warn; use crate::internal::paths::WriterKey; use crate::internal::state::{HollowBatch, RunId}; @@ -307,7 +308,18 @@ impl Trace { |id: SpineId, expected_desc: Option<&Description>| -> Result<_, String> { if let Some(batch) = hollow_batches.remove(&id) { if let Some(desc) = expected_desc { - assert_eq!(*desc, batch.desc); + // We don't expect the desc's upper and lower to change for a given spine id. + assert_eq!(desc.lower(), batch.desc.lower()); + assert_eq!(desc.upper(), batch.desc.upper()); + // Due to the way thin spine batches are diffed, the sinces can be out of sync. + // This should be rare, and hopefully impossible once we change how diffs work. + if desc.since() != batch.desc.since() { + warn!( + "unexpected since out of sync for spine batch: {:?} != {:?}", + desc.since().elements(), + batch.desc.since().elements() + ); + } } return Ok(IdHollowBatch { id, batch }); }