From aaffac26d96270fdab9abc101ffe4089baba7c75 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Tue, 22 Sep 2026 08:23:55 -0400 Subject: [PATCH] feat(pds)!: drop every reader kept for a pre-launch data directory Entry::AccountPinned and Entry::RecordPut, the pinned and soft-deleted aliases, the compatibility defaults on HostedAccount and RepoCommitted, LedgerEvent::Provisioned.node_id, EdgeKind::Attested, the bare pds.wal adopted as segment zero, and the adopted and backfilled stamps. A directory that holds state and carries no stamp now refuses with its own message; the blob index takes a RecordBody where it took a log entry. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: I1b362a8b80c053e918e77a45f306a8daaaa4b075 --- crates/didbot-pds/src/account.rs | 103 ++-------- crates/didbot-pds/src/blobs.rs | 44 ++-- crates/didbot-pds/src/durable.rs | 51 +---- crates/didbot-pds/src/journal.rs | 36 +++- crates/didbot-pds/src/kind.rs | 68 +----- crates/didbot-pds/src/layout.rs | 194 ++++++------------ crates/didbot-pds/src/ledger.rs | 17 +- crates/didbot-pds/src/provision/registry.rs | 1 - .../src/provision/server_account.rs | 1 - crates/didbot-pds/src/records.rs | 139 +++---------- crates/didbot-pds/src/registration.rs | 1 - crates/didbot-pds/src/wal/mod.rs | 163 ++------------- crates/didbot-pds/tests/durability.rs | 52 ++--- crates/didbot-pds/tests/ledger.rs | 2 - crates/didbot-pds/tests/upgrade.rs | 106 +++++----- crates/didbot-serve/src/tests/mod.rs | 1 - crates/didbot/tests/scenarios/restart.rs | 7 +- docs/operations.md | 18 +- docs/running-locally.md | 4 +- 19 files changed, 303 insertions(+), 705 deletions(-) diff --git a/crates/didbot-pds/src/account.rs b/crates/didbot-pds/src/account.rs index 6e238ae9..ad1d1e25 100644 --- a/crates/didbot-pds/src/account.rs +++ b/crates/didbot-pds/src/account.rs @@ -128,13 +128,11 @@ pub enum AccountState { /// history stays verifiable — and the zone and the name stay with it: a /// `did:web` identity cannot leave the domain that hosts /// it. The only further move is a hard delete, which takes the row out. - #[serde(alias = "soft-deleted")] Decommissioned, } impl Default for AccountState { - /// [`AccountState::Active`], so a record written before this field - /// existed deserializes as an account nothing has happened to. + /// [`AccountState::Active`]: an account nothing has happened to. fn default() -> Self { Self::Active } @@ -348,15 +346,11 @@ pub struct SyncAccountStatus { /// logged, listed or serialized without any path by which the secret comes /// with it. /// -/// # The pin that used to be a field +/// # The pin is a retention /// -/// `pinned` was a boolean here standing in for a retention policy. It is -/// [`Retention::Forever`] now and a method rather than a field, because two -/// mechanisms for one decision can disagree and the one that lost would still -/// be the one some caller was reading. A record written before retention -/// existed still carries `pinned` and no `retention`; the field below reads that -/// name too, and [`Retention`]'s deserializer accepts the boolean that was -/// under it. +/// Pinning is [`Retention::Forever`], and a method rather than a field, +/// because two mechanisms for one decision can disagree and the one that +/// lost would still be the one some caller was reading. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub struct HostedAccount { @@ -382,14 +376,12 @@ pub struct HostedAccount { /// machine. Kept beside the values rather than replacing them, because the /// kind names a *row of defaults* and the account carries what it actually /// holds — see [`AccountKind`]. - #[serde(default)] pub kind: AccountKind, /// Whether this server picked the name or the caller asserted it. /// /// Indistinguishable after the fact from the handle alone, and the /// difference decides whether the name goes back into the pool when the /// account does. - #[serde(default)] pub name_provenance: NameProvenance, /// How long the account keeps resolving once it stops being used. /// @@ -398,9 +390,6 @@ pub struct HostedAccount { /// refuses an account held forever rather than leaving the protection to /// callers to remember. /// - /// Aliased to `pinned` on the way in, so an account written down before - /// retention existed keeps its pin instead of quietly becoming sweepable. - #[serde(default, alias = "pinned")] pub retention: Retention, /// The hosted account that created this one, and whose repository holds /// the `bot.did.operator` record for it. @@ -420,13 +409,6 @@ pub struct HostedAccount { )] pub parent: Option, /// Whether the data exists; see [`AccountState`]. - /// - /// Defaults to [`AccountState::Active`] on deserialize, so an account - /// written down before this field existed reads back as one nothing has - /// happened to — the correct reading, since every account this field - /// could be missing from predates a state machine that could have moved - /// it out of that state. - #[serde(default)] pub state: AccountState, /// Every lock hung on the account; see [`crate::lockout`]. /// @@ -1043,7 +1025,6 @@ impl crate::journal::Journaled for MemoryAccountStore { entry, crate::wal::Entry::AccountInserted { .. } | crate::wal::Entry::AccountRemoved { .. } - | crate::wal::Entry::AccountPinned { .. } | crate::wal::Entry::AccountRetained { .. } | crate::wal::Entry::AccountStateChanged { .. } | crate::wal::Entry::AccountLocked { .. } @@ -1066,21 +1047,6 @@ impl crate::journal::Journaled for MemoryAccountStore { } } crate::wal::Entry::AccountRemoved { did } => self.force_remove(&did), - // Written by a build that predates retention. `true` was "never - // collect this" and `false` was "collect it on the deployment's - // own schedule", which are two of the retentions an account can - // hold. - crate::wal::Entry::AccountPinned { did, pinned } => { - if let Ok(did) = AccountDid::parse(&did).map(HostedDid::replayed) { - let retention = if pinned { - Retention::Forever - } else { - self.get(&did) - .map_or(Retention::default(), |account| account.kind.retention()) - }; - let _ = self.set_retention(&did, retention); - } - } crate::wal::Entry::AccountRetained { did, retention } => { if let Ok(did) = AccountDid::parse(&did).map(HostedDid::replayed) { let _ = self.set_retention(&did, retention); @@ -1174,51 +1140,22 @@ mod tests { } } - /// The one direction a dropped field must not fail in: a pinned account - /// written to the log before retention existed must not come back - /// sweepable. - #[test] - fn a_log_written_before_retention_keeps_its_pin() { - let mut written = serde_json::to_value(account()).expect("serializes"); - let object = written.as_object_mut().expect("an account is an object"); - object.remove("retention"); - object.remove("kind"); - object.remove("nameProvenance"); - object.insert("pinned".to_owned(), serde_json::Value::Bool(true)); - - let read: HostedAccount = serde_json::from_value(written).expect("deserializes"); - assert_eq!(read.retention, Retention::Forever); - assert!(read.pinned(), "the pin must survive the rename"); - assert_eq!(read.kind, AccountKind::Agent); - } - - /// And the other way: an unpinned account is one the deployment's own - /// sweep window applies to, which is what `false` meant. - #[test] - fn a_log_written_before_retention_keeps_its_absence_of_a_pin() { - let mut written = serde_json::to_value(account()).expect("serializes"); - let object = written.as_object_mut().expect("an account is an object"); - object.remove("retention"); - object.insert("pinned".to_owned(), serde_json::Value::Bool(false)); - - let read: HostedAccount = serde_json::from_value(written).expect("deserializes"); - assert_eq!(read.retention, Retention::Deployment); - assert!(!read.pinned()); - } - - /// No `pinned` and no `retention` at all — every account this deployment - /// writes from here on carries `retention`, so this is only the shape a - /// hand-edited or truncated record could have. + /// Every field an account carries is required on the way back in. + /// + /// A missing one is a record this binary cannot read rather than one it + /// fills in, because a filled-in `retention` is an account that comes + /// back sweepable and a filled-in `state` is one that comes back active. #[test] - fn an_account_with_neither_reads_as_the_deployments_own_schedule() { - let mut written = serde_json::to_value(account()).expect("serializes"); - written - .as_object_mut() - .expect("an account is an object") - .remove("retention"); - - let read: HostedAccount = serde_json::from_value(written).expect("deserializes"); - assert_eq!(read.retention, Retention::Deployment); + fn an_account_missing_a_field_is_refused_rather_than_defaulted() { + for field in ["retention", "kind", "nameProvenance", "state"] { + let mut written = serde_json::to_value(account()).expect("serializes"); + written + .as_object_mut() + .expect("an account is an object") + .remove(field); + let read: Result = serde_json::from_value(written); + assert!(read.is_err(), "a missing `{field}` read back as an account"); + } } /// The public half is answerable on its own, and it is the same key the diff --git a/crates/didbot-pds/src/blobs.rs b/crates/didbot-pds/src/blobs.rs index f1057b7c..764f1cc4 100644 --- a/crates/didbot-pds/src/blobs.rs +++ b/crates/didbot-pds/src/blobs.rs @@ -1566,9 +1566,9 @@ impl crate::journal::Derived for crate::FileBlobStore { /// a time. /// /// This runs before the record store applies the entry, and that is what - /// makes it correct: the record a `RecordPut` is about to overwrite is - /// still readable here, and the blobs *it* pointed at are the ones that - /// stop being live. Counting after the overwrite would leave a blob an + /// makes it correct: the record a write is about to overwrite is still + /// readable here, and the blobs *it* pointed at are the ones that stop + /// being live. Counting after the overwrite would leave a blob an /// earlier version of the record pointed at looking referenced forever. /// /// Incremental, and against the blobs the account holds as the entry is @@ -1584,24 +1584,6 @@ impl crate::journal::Derived for crate::FileBlobStore { /// what maintains the count over everything the journal holds after it. fn observe(&self, entry: &Entry, from: crate::journal::Stores<'_>) { match entry { - Entry::RecordPut { - did, - collection, - rkey, - record, - } => { - let replaced = from.records.get(did, collection, rkey); - let new_refs = crate::records::blob_refs(record); - if let Some(replaced) = replaced { - let old_refs = crate::records::blob_refs(&replaced); - if !old_refs.is_empty() { - self.unmark_referenced(did, &old_refs); - } - } - if !new_refs.is_empty() { - self.mark_referenced(did, &new_refs); - } - } // A tombstone is a delete too: the body stays readable by name // for the policy gate, and it is not a record that references // anything. @@ -1629,6 +1611,26 @@ impl crate::journal::Derived for crate::FileBlobStore { _ => {} } } + + fn observe_record(&self, body: &crate::journal::RecordBody, from: crate::journal::Stores<'_>) { + let crate::journal::RecordBody { + did, + collection, + rkey, + record, + } = body; + let replaced = from.records.get(did, collection, rkey); + let new_refs = crate::records::blob_refs(record); + if let Some(replaced) = replaced { + let old_refs = crate::records::blob_refs(&replaced); + if !old_refs.is_empty() { + self.unmark_referenced(did, &old_refs); + } + } + if !new_refs.is_empty() { + self.mark_referenced(did, &new_refs); + } + } } #[cfg(test)] diff --git a/crates/didbot-pds/src/durable.rs b/crates/didbot-pds/src/durable.rs index 4e0145ce..65f24466 100644 --- a/crates/didbot-pds/src/durable.rs +++ b/crates/didbot-pds/src/durable.rs @@ -170,19 +170,6 @@ impl Durable { let lock = crate::lock::DirLock::acquire(dir)?; match crate::layout::check(dir, has_state)? { crate::layout::Found::Fresh => {} - crate::layout::Found::Adopted => tracing::info!( - layout = crate::layout::LAYOUT, - shape = %format!("{:016x}", crate::layout::SHAPE), - "the data directory predates layout stamping; adopting it" - ), - crate::layout::Found::Backfilled { from_layout } => tracing::warn!( - from_layout, - layout = crate::layout::LAYOUT, - shape = %format!("{:016x}", crate::layout::SHAPE), - names = %format!("{:016x}", crate::layout::names()), - "the data directory's stamp predates the on-disk name manifest and its entry \ - shape agrees; backfilling the names half and continuing" - ), crate::layout::Found::Matched => tracing::debug!( layout = crate::layout::LAYOUT, shape = %format!("{:016x}", crate::layout::SHAPE), @@ -611,12 +598,12 @@ fn apply( entry: Entry, ) { // Derived state first, and against the stores as they stand: the blob - // index's reference counts are taken over the record a `RecordPut` is - // about to replace, and that record is only readable until the record - // store applies the entry. See the [`Derived`] implementation in + // index's reference counts are taken over the record a write is about to + // replace, and that record is only readable until the record store + // applies the entry. See the [`Derived`] implementation in // [`crate::blobs`]. - // A stored record reaches the blob index as the put it is. The index - // counts the blobs a record names by reading the record, and a + // + // The index counts the blobs a record names by reading the record, and a // `RecordStored` carries a slot instead — so the body is resolved out of // the heap here and the index is handed the fact in the terms it holds // counts in. @@ -636,8 +623,8 @@ fn apply( .. } if blobs.index().holds_blobs(did) => { if let Some(record) = records.replayed_body(&entry) { - blobs.observe( - &Entry::RecordPut { + blobs.observe_record( + &crate::journal::RecordBody { did: did.clone(), collection: collection.clone(), rkey: rkey.clone(), @@ -2761,8 +2748,8 @@ mod tests { /// /// * A record only ever references blobs already uploaded under the same /// DID. `FileBlobStore::commit` renames the bytes and appends - /// `BlobUploaded` before the reference can be written, so a `RecordPut` - /// never precedes the upload it names. + /// `BlobUploaded` before the reference can be written, so a record + /// write never precedes the upload it names. /// * A blob is only collected while no live record references it. /// `collect_unreferenced` takes candidates from /// `BlobIndex::collect_candidates`, which is exactly the blobs at zero @@ -2813,13 +2800,7 @@ mod tests { } } 1 => Entry::AccountRemoved { did }, - // The variant no build writes any more, whose effect a - // compaction has to carry as a retention. - 2 => Entry::AccountPinned { - did, - pinned: seq.pick(2) == 0, - }, - 3 => Entry::AccountRetained { + 2 | 3 => Entry::AccountRetained { did, retention: *seq.of(&[ Retention::Forever, @@ -2850,17 +2831,7 @@ mod tests { let record = record_over(&refs); let key = (did.clone(), collection.clone(), rkey.clone()); written.insert(key.clone(), refs); - if seq.pick(2) == 0 { - // The variant no build writes any more, whose body - // a replay moves into the heap itself. - stored.remove(&key); - Entry::RecordPut { - did, - collection, - rkey, - record, - } - } else { + { let (body, cid) = crate::records::encode_record(&record).expect("a record encodes"); let slot = heap diff --git a/crates/didbot-pds/src/journal.rs b/crates/didbot-pds/src/journal.rs index 5f64420b..8f7fce96 100644 --- a/crates/didbot-pds/src/journal.rs +++ b/crates/didbot-pds/src/journal.rs @@ -45,6 +45,8 @@ use std::path::Path; +use serde_json::Value; + use crate::heap::Heap; use crate::records::RecordStore; use crate::wal::Entry; @@ -191,6 +193,24 @@ pub enum Bodies<'a> { Heap(&'a Heap), } +/// One record's body, under the key it was written at. +/// +/// What a [`Derived`] store is handed for a record write. The journal +/// carries a locator and the record store keeps the value in its heap, so +/// the value is resolved out of the heap first and the derivation is taken +/// over a record rather than over an entry. +#[derive(Debug, Clone)] +pub struct RecordBody { + /// The repository. + pub did: String, + /// The collection. + pub collection: String, + /// The key it was written under. + pub rkey: String, + /// The record. + pub record: Value, +} + /// A store whose state is a function of another store's. /// /// The blob index's reference counts are the case: they are the number of @@ -206,11 +226,19 @@ pub trait Derived: Journaled { /// it. /// /// Called for every replayed entry in journal order, with `from` as the - /// stores stand before `entry` is applied. That is what a count taken - /// over a replaced value needs: the record a `RecordPut` overwrites is - /// still readable when this runs, and it is the references it held that - /// stop being live. + /// stores stand before `entry` is applied. fn observe(&self, entry: &Entry, from: Stores<'_>); + + /// Reads one record's body, before the record store applies the entry + /// that named it. + /// + /// Separate from [`Derived::observe`] because the journal names a + /// record's body rather than carrying it, so the caller resolves the + /// value first. `from` is the stores as they stand before the write + /// lands, which is what a count taken over a replaced value needs: the + /// record this one overwrites is still readable here, and it is the + /// references *it* held that stop being live. + fn observe_record(&self, body: &RecordBody, from: Stores<'_>); } /// Replays a store's own checkpoint into a fresh one, for a store's diff --git a/crates/didbot-pds/src/kind.rs b/crates/didbot-pds/src/kind.rs index 383152cf..51ff68db 100644 --- a/crates/didbot-pds/src/kind.rs +++ b/crates/didbot-pds/src/kind.rs @@ -149,10 +149,9 @@ impl NameProvenance {} /// How long an account keeps resolving after it stops being used. /// -/// This is the one mechanism. The pin that used to be a boolean on the -/// account is [`Retention::Forever`] and nothing else — two mechanisms for -/// one decision could disagree, and the one that lost would still be the one -/// some caller was reading. +/// This is the one mechanism, and a pin is [`Retention::Forever`] — two +/// mechanisms for one decision could disagree, and the one that lost would +/// still be the one some caller was reading. /// /// # What "stops being used" means /// @@ -164,7 +163,7 @@ impl NameProvenance {} /// such moment to read; see the module documentation. Until there is, the /// kinds that would renew hold [`Retention::Forever`] rather than a duration /// measured against a clock that does not describe them. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub enum Retention { /// Swept once it has been idle longer than the deployment's own sweep @@ -185,59 +184,22 @@ pub enum Retention { /// machine's retention is whatever that machine's lifetime turns out to /// be, and the kind cannot know it. Until { - /// The window, in whole seconds. Seconds rather than a `Duration` - /// because this is serialized into the write-ahead log, and `time`'s - /// serde support is not enabled in this workspace. + /// The window, in whole seconds. A number rather than a + /// `time::Duration` because this is serialized into the write-ahead + /// log, where a window is a count of seconds and nothing else. seconds: i64, }, /// Never swept. /// - /// What the pin was. [`AccountStore::remove`] refuses an account holding + /// What a pin is. [`AccountStore::remove`] refuses an account holding /// this rather than leaving the protection to callers to remember. /// /// [`AccountStore::remove`]: crate::AccountStore::remove Forever, } -/// Reads a retention, or the boolean pin that used to stand in for one. -/// -/// `HostedAccount` aliases this field to `pinned`, so an account written to the -/// write-ahead log before retention existed deserializes through here with a -/// `true` or a `false` where the enum now goes. `true` was "never collect -/// this", which is [`Retention::Forever`]; `false` was "collect it on the -/// deployment's own schedule", which is [`Retention::Deployment`]. Dropping -/// the boolean instead would silently unpin every pinned account across one -/// restart, which is the one direction this must not fail in. -impl<'de> serde::Deserialize<'de> for Retention { - fn deserialize>(de: D) -> Result { - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - enum Written { - Deployment, - Until { seconds: i64 }, - Forever, - } - - #[derive(serde::Deserialize)] - #[serde(untagged)] - enum Either { - LegacyPin(bool), - Written(Written), - } - - Ok(match Either::deserialize(de)? { - Either::LegacyPin(true) => Self::Forever, - Either::LegacyPin(false) => Self::Deployment, - Either::Written(Written::Deployment) => Self::Deployment, - Either::Written(Written::Until { seconds }) => Self::Until { seconds }, - Either::Written(Written::Forever) => Self::Forever, - }) - } -} - impl Default for Retention { - /// [`Retention::Deployment`], which is what an unpinned account written - /// down before retention existed was subject to: the sweep's own window. + /// [`Retention::Deployment`]: the sweep's own window. fn default() -> Self { Self::Deployment } @@ -347,18 +309,6 @@ mod tests { ); } - #[test] - fn the_boolean_pin_still_reads_as_a_retention() { - assert_eq!( - serde_json::from_str::("true").expect("a legacy pin"), - Retention::Forever - ); - assert_eq!( - serde_json::from_str::("false").expect("a legacy unpinned account"), - Retention::Deployment - ); - } - #[test] fn a_kind_round_trips_as_its_union_member_name() { assert_eq!( diff --git a/crates/didbot-pds/src/layout.rs b/crates/didbot-pds/src/layout.rs index aba24159..e8e1ffaf 100644 --- a/crates/didbot-pds/src/layout.rs +++ b/crates/didbot-pds/src/layout.rs @@ -110,34 +110,16 @@ //! relative path set, so a new file fails it. The two are complementary and //! neither replaces the other. //! -//! # Migration +//! # A stamp this binary cannot compare //! -//! A stamp from before the derived shape landed has a `layout` field and no -//! `shape` field. It is refused — [`LayoutError::Legacy`] — rather than -//! adopted the way a directory with no stamp at all is: unlike that case, a -//! `layout` integer with no shape behind it cannot be compared against -//! anything, so there is no honest way to call it a match. +//! A stamp carries a `shape` and a `names`. One missing either half has +//! nothing to compare against on that half, so it is refused: +//! [`LayoutError::Legacy`]. A directory holding state and no stamp at all is +//! refused too — [`LayoutError::Unstamped`] — because the alternative is a +//! restore that lost `pds.layout` coming up stamped with whatever binary +//! opened it, silently. An empty directory is stamped and accepted; there is +//! nothing there to be wrong about. //! -//! # Backfilling the names half -//! -//! A stamp with a `shape` and no `names` is the other pre-existing case, and -//! it is not the same one. Its shape *is* comparable, and a shape that -//! matches this binary's exactly says the build that wrote the directory -//! declared the same [`ENTRY_SHAPE`] — which, in this repository, is only -//! true of builds whose [`did_path`](crate::blobs::did_path), CID rendering -//! and [`NAME_SHAPE`] constants are the same code. So the names half is not -//! assumed, it is implied: the files are named what this binary expects, and -//! [`names`] is written into the stamp rather than the directory being -//! refused. [`Found::Backfilled`] says so at a level an operator sees. -//! -//! The risk runs one way and it is worth naming. Writing the *current* -//! `names` into an old stamp asserts something about bytes already on disk -//! that nothing on disk records. That assertion is only carried by the shape -//! comparison, so the shape must match *exactly* — a stamp whose shape -//! differs is refused, unchanged, because then nothing is known about either -//! half. Weakening that comparison would turn a proof into a hope, and the -//! thing being hoped about is `FileBlobStore::sweep` deleting every blob. - use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -147,7 +129,7 @@ use serde::{Deserialize, Serialize}; /// This is **not** what decides whether a directory is accepted — see the /// module docs. Bump it, or don't; a stale or colliding value only makes a /// refusal's message less helpful, never wrong. -pub const LAYOUT: u32 = 17; +pub const LAYOUT: u32 = 18; /// Every [`Entry`](crate::wal::Entry) variant, and the fields it writes on /// the wire, sorted by variant name and then by field name. @@ -162,7 +144,6 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ ("accountInserted", &["account", "key", "op"]), ("accountLocked", &["did", "lock", "op", "party"]), ("accountParented", &["did", "op", "parent"]), - ("accountPinned", &["did", "op", "pinned"]), ("accountRemoved", &["did", "op"]), ("accountRetained", &["did", "op", "retention"]), ("accountStateChanged", &["did", "op", "state"]), @@ -211,7 +192,6 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ "refresh_hash", ], ), - ("recordPut", &["collection", "did", "op", "record", "rkey"]), ("recordRemoved", &["collection", "did", "op", "rkey"]), ( "recordStored", @@ -426,42 +406,11 @@ pub struct Stamp { pub names: u64, } -/// A stamp from before the on-disk names were covered. -/// -/// The same fields [`Stamp`] had then. Read separately rather than by giving -/// [`Stamp::names`] a serde default, because a default would make an absent -/// `names` indistinguishable from one that happens to equal it, and the -/// whole point is to treat the two differently. -#[derive(Debug, Clone, Deserialize)] -struct PreNameStamp { - /// The human-legible sequence the directory was written under. - layout: u32, - /// The derived entry-shape hash. Compared, and required to match. - shape: u64, -} - /// What opening a directory found. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Found { /// Nothing was here. A stamp was written. Fresh, - /// A log was here with no stamp, from before stamping existed. Adopted. - /// - /// Adopted rather than refused, because every data directory in existence - /// when this landed is one of these, and refusing them all would make the - /// feature's first act be to break every developer's state. - Adopted, - /// A stamp was here with a `shape` that agrees and no `names` half, - /// from before the on-disk names were covered. The names half was - /// backfilled and the stamp rewritten. - /// - /// See the module docs' "Backfilling the names half" for why the - /// agreeing shape is what makes this sound rather than optimistic. - Backfilled { - /// The human-legible sequence the old stamp carried, so a log line - /// can say which one was adopted rather than only that one was. - from_layout: u32, - }, /// A stamp was here and it agrees with this binary. Matched, } @@ -527,7 +476,7 @@ pub enum LayoutError { /// This binary's derived on-disk names. expected_names: u64, }, - /// The stamp predates the derived shape check and cannot be compared. + /// The stamp is missing a half, so there is nothing to compare it with. #[error( "the data directory at {path} carries a stamp written before this binary's layout \ check existed, so half of it has nothing to compare against. Delete the directory \ @@ -537,6 +486,22 @@ pub enum LayoutError { /// The directory. path: PathBuf, }, + /// The directory holds state and says nothing about what wrote it. + /// + /// Stamping it with this binary's own layout would be asserting something + /// about bytes on disk that nothing on disk records — a restore that lost + /// `pds.layout` would come up looking healthy and be misread. + #[error( + "the data directory at {path} holds state and carries no {stamp} saying what wrote \ + it, so nothing here can say whether this binary reads it. Restore the {stamp} \ + alongside the rest of the directory, or delete the directory and start again" + )] + Unstamped { + /// The directory. + path: PathBuf, + /// The stamp file, by name. + stamp: &'static str, + }, /// The stamp is there and is not a stamp. #[error("the stamp at {path} is not usable: {source}")] Unreadable { @@ -562,34 +527,13 @@ pub fn check(dir: &Path, has_state: bool) -> Result { path: path.clone(), source: Box::new(source), })?; - // A stamp from before the derived shape landed has no `shape` - // field. There is nothing to compare it against on either half, - // so it is refused — see the module docs' "Migration" section. - if value.get("shape").is_none() { + // A stamp missing either half has nothing to compare against on + // that half, so there is no honest way to call it a match. + if value.get("shape").is_none() || value.get("names").is_none() { return Err(LayoutError::Legacy { path: dir.to_path_buf(), }); } - // A stamp with a `shape` and no `names` was written before the - // on-disk names were covered. Its shape is comparable, and a - // shape that matches is what makes the missing half knowable - // rather than assumed — see "Backfilling the names half". - if value.get("names").is_none() { - let stamp: PreNameStamp = - serde_json::from_value(value).map_err(|source| LayoutError::Unreadable { - path: path.clone(), - source: Box::new(source), - })?; - if stamp.shape != SHAPE { - return Err(LayoutError::Legacy { - path: dir.to_path_buf(), - }); - } - write(&path)?; - return Ok(Found::Backfilled { - from_layout: stamp.layout, - }); - } let stamp: Stamp = serde_json::from_value(value).map_err(|source| LayoutError::Unreadable { path: path.clone(), @@ -617,12 +561,14 @@ pub fn check(dir: &Path, has_state: bool) -> Result { Ok(Found::Matched) } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + if has_state { + return Err(LayoutError::Unstamped { + path: dir.to_path_buf(), + stamp: STAMP_FILE, + }); + } write(&path)?; - Ok(if has_state { - Found::Adopted - } else { - Found::Fresh - }) + Ok(Found::Fresh) } Err(source) => Err(LayoutError::Unreadable { path, @@ -669,11 +615,20 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + /// A directory that holds something and says nothing about what wrote + /// it is refused. Stamping it would be this binary asserting a shape + /// over bytes it has not read. #[test] - fn a_directory_from_before_stamping_is_adopted_rather_than_refused() { - // Every data directory in existence when this landed is one of these. - let dir = scratch("adopt"); - assert_eq!(check(&dir, true).expect("adopted"), Found::Adopted); + fn a_directory_with_state_and_no_stamp_is_refused() { + let dir = scratch("unstamped"); + let err = check(&dir, true).expect_err("an unstamped directory with state is refused"); + assert!(matches!(err, LayoutError::Unstamped { .. }), "{err}"); + assert!( + !dir.join(STAMP_FILE).exists(), + "the refusal must not stamp the directory" + ); + let rendered = err.to_string(); + assert!(rendered.contains(STAMP_FILE), "{rendered}"); std::fs::remove_dir_all(&dir).ok(); } @@ -704,7 +659,7 @@ mod tests { } #[test] - fn a_stamp_from_before_shape_derivation_is_refused_rather_than_adopted() { + fn a_stamp_from_before_shape_derivation_is_refused() { let dir = scratch("legacy"); // The format every stamp had before `shape` existed. std::fs::write(dir.join(STAMP_FILE), r#"{"layout": 4}"#).expect("write a stamp"); @@ -714,52 +669,27 @@ mod tests { } #[test] - fn a_stamp_from_before_the_names_half_is_backfilled_rather_than_refused() { - let dir = scratch("backfill"); - // The exact format a stamp had between the derived shape landing and - // the names half landing. `SHAPE` is the real one, because that is - // the whole premise: the shape agrees, so the names are implied. + fn a_stamp_missing_the_names_half_is_refused() { + let dir = scratch("half-stamp"); + // A stamp carrying a shape and no names. Its shape agreeing says + // nothing about where the blobs are, so there is nothing to compare. std::fs::write( dir.join(STAMP_FILE), format!(r#"{{"layout": {}, "shape": {}}}"#, LAYOUT - 1, SHAPE), ) .expect("write a stamp"); - - assert_eq!( - check(&dir, true).expect("an agreeing shape backfills"), - Found::Backfilled { - from_layout: LAYOUT - 1 - } - ); - - // Backfilled means the stamp on disk was rewritten, not that the - // check quietly returned. Read it back as a whole `Stamp`, which a - // stamp with no `names` cannot deserialize as. - let raw = std::fs::read_to_string(dir.join(STAMP_FILE)).expect("the stamp is readable"); - let stamp: Stamp = serde_json::from_str(&raw).expect("a full stamp was written"); - assert_eq!( - stamp, - Stamp { - layout: LAYOUT, - shape: SHAPE, - names: names(), - } - ); - - // And the next boot is an ordinary one. - assert_eq!(check(&dir, true).expect("second open"), Found::Matched); + let err = check(&dir, true).expect_err("half a stamp cannot be compared"); + assert!(matches!(err, LayoutError::Legacy { .. }), "{err}"); std::fs::remove_dir_all(&dir).ok(); } - /// The half that makes the backfill sound rather than optimistic. + /// A refused stamp is left exactly as it was found. /// - /// A shape that does not match says nothing is known about the log, and - /// therefore nothing is known about the names either. Writing this - /// binary's `names` into that stamp would be asserting where the blobs - /// are on the strength of no evidence at all. + /// A refusal that rewrote the stamp on its way out would leave the next + /// boot an ordinary one, which is the refusal undone. #[test] - fn a_stamp_from_before_the_names_half_with_another_shape_is_still_refused() { - let dir = scratch("backfill-foreign"); + fn a_refused_stamp_is_not_rewritten() { + let dir = scratch("half-stamp-foreign"); let stamp = dir.join(STAMP_FILE); let raw = format!( r#"{{"layout": {}, "shape": {}}}"#, @@ -771,8 +701,6 @@ mod tests { let err = check(&dir, true).expect_err("a foreign shape is refused"); assert!(matches!(err, LayoutError::Legacy { .. }), "{err}"); - // A refusal that rewrote the stamp on its way out would leave the - // directory adopted by the next boot, which is the refusal undone. assert_eq!( std::fs::read_to_string(&stamp).expect("the stamp is readable"), raw, diff --git a/crates/didbot-pds/src/ledger.rs b/crates/didbot-pds/src/ledger.rs index e63a3475..dbdd6d01 100644 --- a/crates/didbot-pds/src/ledger.rs +++ b/crates/didbot-pds/src/ledger.rs @@ -49,9 +49,8 @@ use crate::account::StoreError; /// it. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] // `rename_all` renames the variants and `rename_all_fields` renames the -// fields inside them. Both are needed: without the second, `accountId` and -// `nodeId` would go out as `account_id` and `node_id`, and every other JSON this -// server emits is camelCase. +// fields inside them. Both are needed: without the second, `accountId` would +// go out as `account_id`, and every JSON this server emits is camelCase. #[serde( tag = "event", rename_all = "camelCase", @@ -75,10 +74,6 @@ pub enum LedgerEvent { /// [`crate::CREATOR_ASSURANCE`]: the caller authenticated the /// creator before the request reached the registry. assurance: String, - /// Carried for entries written before creators existed; nothing - /// writes it now. - #[serde(default, skip_serializing_if = "Option::is_none")] - node_id: Option, /// The account that spawned this one, if this deployment hosts it. /// /// A fact the server checked rather than one it was told: an @@ -223,8 +218,6 @@ impl LedgerEvent { pub enum EdgeKind { /// A `bot.did.operator` record in the parent's repository. Vouched, - /// Evidence a since-removed attestation backend verified. - Attested, } /// One entry: what happened, when, and where it sits in the sequence. @@ -603,9 +596,8 @@ mod tests { fn provisioned() -> LedgerEvent { LedgerEvent::Provisioned { account_id: "quartz-vole".to_owned(), - backend: "dev-shared-secret".to_owned(), - assurance: "shared-secret".to_owned(), - node_id: Some("node-7".to_owned()), + backend: "server".to_owned(), + assurance: crate::CREATOR_ASSURANCE.to_owned(), parent: None, } } @@ -800,7 +792,6 @@ mod tests { }; let json = serde_json::to_string(&entry).expect("an entry serializes"); assert!(json.contains("\"accountId\""), "{json}"); - assert!(json.contains("\"nodeId\""), "{json}"); assert!(!json.contains('_'), "no snake_case anywhere: {json}"); let back: LedgerEntry = serde_json::from_str(&json).expect("and reads back"); assert_eq!(back, entry); diff --git a/crates/didbot-pds/src/provision/registry.rs b/crates/didbot-pds/src/provision/registry.rs index 05335cc2..964f858d 100644 --- a/crates/didbot-pds/src/provision/registry.rs +++ b/crates/didbot-pds/src/provision/registry.rs @@ -244,7 +244,6 @@ where account_id: account_id.clone(), backend: request.creator.label().to_owned(), assurance: CREATOR_ASSURANCE.to_owned(), - node_id: None, parent: Some(operator.clone()), }, ) { diff --git a/crates/didbot-pds/src/provision/server_account.rs b/crates/didbot-pds/src/provision/server_account.rs index e906e1bc..d3d3d849 100644 --- a/crates/didbot-pds/src/provision/server_account.rs +++ b/crates/didbot-pds/src/provision/server_account.rs @@ -92,7 +92,6 @@ where account_id: "server".to_owned(), backend: "self-provisioned".to_owned(), assurance: CREATOR_ASSURANCE.to_owned(), - node_id: None, parent: Some(self.operator.clone()), }, ); diff --git a/crates/didbot-pds/src/records.rs b/crates/didbot-pds/src/records.rs index 8f281a04..355ba7ab 100644 --- a/crates/didbot-pds/src/records.rs +++ b/crates/didbot-pds/src/records.rs @@ -1399,30 +1399,6 @@ impl MemoryRecordStore { .insert(did, collection, rkey.to_owned(), record); } } - - /// Every record it holds, as `(did, collection, rkey, record)`. - /// - /// In repository, collection and key order, which is the order the maps - /// are already in. Compaction writes them back in this order rather than - /// the order they were originally written; nothing depends on the - /// difference, because a key says when its record was written. - pub(crate) fn drain_snapshot(&self) -> Vec<(String, String, String, Value)> { - let repos = self.repos(); - let mut out = Vec::new(); - for (did, repo) in repos.iter() { - for (collection, records) in repo { - for (rkey, record) in records { - out.push(( - did.clone(), - collection.clone(), - rkey.clone(), - record.clone(), - )); - } - } - } - out - } } /// The CID of whatever is at a key, for a precondition to be checked against. @@ -1622,59 +1598,6 @@ impl RecordStore for MemoryRecordStore { } } -impl crate::journal::Journaled for MemoryRecordStore { - const STORE: &'static str = "records"; - - fn owns(entry: &Entry) -> bool { - matches!(entry, Entry::RecordPut { .. } | Entry::RecordRemoved { .. }) - } - - fn apply(&self, entry: Entry) { - match entry { - Entry::RecordPut { - did, - collection, - rkey, - record, - } => self.insert_at(&did, &collection, rkey, record), - Entry::RecordRemoved { - did, - collection, - rkey, - } => { - // Replay is not conditional on anything: the log is a record - // of what already happened, and a precondition that was - // checked at the time is not checked again. The guard's - // refusal is discarded for the same reason — a log entry is a - // deletion that already happened and was acknowledged, and - // re-running the rule against it would let a tightened guard - // resurrect a record a previous version of this server - // deleted. - let _ = self.remove(&did, &collection, &rkey, &Precondition::Unconditional); - } - // Only what `owns` claims reaches here. - _ => {} - } - } - - /// One entry per record, under the key it was written with. - fn checkpoint(&self) -> Vec { - self.drain_snapshot() - .into_iter() - .map(|(did, collection, rkey, record)| Entry::RecordPut { - did, - collection, - rkey, - record, - }) - .collect() - } - - fn durable_through(&self) -> crate::journal::Mark { - crate::journal::Mark::ORIGIN - } -} - #[cfg(test)] mod tombstone_tests { use super::*; @@ -1866,13 +1789,35 @@ mod tombstone_tests { mod journal_tests { use super::*; - fn put(store: &MemoryRecordStore, did: &str, collection: &str, rkey: &str, text: &str) { - store.insert_at( - did, - collection, - rkey.to_owned(), - serde_json::json!({ "$type": collection, "text": text }), - ); + /// The store the deployment runs, over a heap of its own. + /// + /// A checkpoint names where each body already is, so the store a + /// checkpoint is replayed into has to be over the heap it was taken + /// from — which is the deployment's case too: a boot reads the heap it + /// found and the checkpoint that describes it. + fn over_a_heap(name: &str) -> (Arc, HeapRecordStore) { + let dir = std::env::temp_dir().join(format!( + "didbot-record-journal-{name}-{}-{}", + std::process::id(), + time::OffsetDateTime::now_utc().unix_timestamp_nanos() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("a scratch directory"); + let heap = Arc::new(Heap::open(&dir).expect("a scratch heap")); + let store = HeapRecordStore::new(heap.clone()); + (heap, store) + } + + fn put(store: &HeapRecordStore, did: &str, collection: &str, rkey: &str, text: &str) { + store + .put( + did, + collection, + Some(rkey), + serde_json::json!({ "$type": collection, "text": text }), + &Precondition::Unconditional, + ) + .expect("the write lands"); } /// The contract's obligation for the largest store: every record comes @@ -1880,7 +1825,7 @@ mod journal_tests { /// collections. #[test] fn a_checkpoint_of_the_records_replays_into_the_same_records() { - let store = MemoryRecordStore::new(); + let (heap, store) = over_a_heap("kept"); put( &store, "did:web:a.example", @@ -1910,7 +1855,7 @@ mod journal_tests { "kestrel", ); - let fresh = MemoryRecordStore::new(); + let fresh = HeapRecordStore::new(heap); let (written, replayed) = crate::journal::round_trip(&store, &fresh); assert_eq!(written, replayed); assert_eq!(fresh.stats().records, 4); @@ -1924,7 +1869,7 @@ mod journal_tests { /// does not carry. #[test] fn a_checkpoint_does_not_carry_a_record_that_was_removed() { - let store = MemoryRecordStore::new(); + let (heap, store) = over_a_heap("removed"); put( &store, "did:web:a.example", @@ -1948,7 +1893,7 @@ mod journal_tests { ) .expect("a stored record"); - let fresh = MemoryRecordStore::new(); + let fresh = HeapRecordStore::new(heap); let (written, replayed) = crate::journal::round_trip(&store, &fresh); assert_eq!(written, replayed); assert_eq!(fresh.stats().records, 1); @@ -2738,7 +2683,6 @@ impl crate::journal::Journaled for HeapRecordStore { entry, Entry::RecordStored { .. } | Entry::RecordTombstoned { .. } - | Entry::RecordPut { .. } | Entry::RecordRemoved { .. } ) } @@ -2773,23 +2717,6 @@ impl crate::journal::Journaled for HeapRecordStore { self.replay_tombstone(&did, &collection, rkey, held); } } - // A log written before the bodies moved. The value is in the - // entry, so it is appended to the heap here and the key gets a - // slot like any other: a directory holding a mixture of the two - // is what every build from the format's own pull onward reads. - Entry::RecordPut { - did, - collection, - rkey, - record, - } => { - if let Err(error) = self.store(&did, &collection, rkey.clone(), &record) { - tracing::error!( - did, collection, rkey, %error, - "a record the log carries in full could not be moved into the heap" - ); - } - } Entry::RecordRemoved { did, collection, diff --git a/crates/didbot-pds/src/registration.rs b/crates/didbot-pds/src/registration.rs index e5ddb0ff..861f564f 100644 --- a/crates/didbot-pds/src/registration.rs +++ b/crates/didbot-pds/src/registration.rs @@ -98,7 +98,6 @@ mod tests { account_id: "kestrel".to_owned(), backend: "server".to_owned(), assurance: "creator".to_owned(), - node_id: None, parent: Some(OPERATOR.to_owned()), } } diff --git a/crates/didbot-pds/src/wal/mod.rs b/crates/didbot-pds/src/wal/mod.rs index 31794417..080d416c 100644 --- a/crates/didbot-pds/src/wal/mod.rs +++ b/crates/didbot-pds/src/wal/mod.rs @@ -136,7 +136,6 @@ use std::sync::Mutex; use std::time::{Duration, Instant}; use didbot_key::SigningKey; -use serde_json::Value; use time::OffsetDateTime; use crate::account::HostedAccount; @@ -160,8 +159,7 @@ pub const DEFAULT_SYNC_INTERVAL: Duration = Duration::from_millis(250); /// share. /// /// The log is written in segments — `pds.wal.000000`, `pds.wal.000001`, and -/// so on, see [`segment_name`] — and a bare `pds.wal` from before it was is -/// adopted as segment zero the first time the directory opens. +/// so on; see [`segment_name`]. pub const LOG_FILE: &str = "pds.wal"; /// Bytes a segment reaches before the next append rolls the log onto a new @@ -359,18 +357,6 @@ pub enum Entry { /// Whose. did: String, }, - /// An account's pin was set or cleared. - /// - /// Replayed, never written. The pin became one setting of - /// [`crate::kind::Retention`], which [`Entry::AccountRetained`] carries; - /// a log written before that still holds these and still has to replay - /// into the same state. - AccountPinned { - /// Whose. - did: String, - /// The new value. - pinned: bool, - }, /// An account's retention was set. /// /// Which is also how an account is pinned and unpinned; see @@ -437,21 +423,6 @@ pub enum Entry { /// The principal it was admitted under. parent: String, }, - /// A record was written. - /// - /// The key is in the entry rather than minted again on replay, because a - /// record's key is part of its `at://` URI: minting a new one would move - /// every record a client had already been told about. - RecordPut { - /// The repository. - did: String, - /// The collection. - collection: String, - /// The key it was written under. - rkey: String, - /// The record. - record: Value, - }, /// One record was deleted. /// /// The whole of the deletion, because a record is addressed by exactly @@ -526,17 +497,14 @@ pub enum Entry { commit: String, /// The root of the tree that commit is over. /// - /// `default` rather than required, so a log written before the - /// firehose needed it still reads: a head restored without one is a - /// head whose first frame after a restart carries no `prevData`. - #[serde(default)] + /// Written as null rather than omitted, so that every entry of this + /// shape has the same fields whichever commit it is. A head restored + /// without one is a head whose first frame after a restart carries no + /// `prevData`. data: Option, /// The commit it replaced, null for a repository's first. /// - /// Written as null rather than omitted, so that every entry of this - /// shape has the same fields whichever commit it is; `default` is - /// what lets a log written without the field still read. - #[serde(default)] + /// Written as null rather than omitted, for the reason `data` is. prev: Option, /// The blocks this commit has that its predecessor did not. /// @@ -700,9 +668,10 @@ pub enum Entry { }, /// A record was written, and its body is in the record store's heap. /// - /// The three names are here for the reason [`Entry::RecordPut`] gives: - /// a record's key is part of its `at://` URI, so it travels rather than - /// being minted again. What is different is the body — the log carries + /// The three names are in the entry rather than minted again on replay, + /// because a record's key is part of its `at://` URI: minting a new one + /// would move every record a client had already been told about. The + /// body is what is different — the log carries /// where the bytes are and what they hash to, and the bytes themselves /// are a frame in `/records/pds.heap` whose payload is the record's /// canonical DAG-CBOR. The ordering is [`Entry::BlobUploaded`]'s: the @@ -912,8 +881,7 @@ pub enum WalError { /// A segment between the position a boot resumes from and the last on /// disk is absent or ends in a frame that does not check out, the /// position to resume from is in a segment that was trimmed against a - /// checkpoint this directory no longer reads, or a `pds.wal` from before - /// the log was segmented sits beside segments. Each is an entry the log + /// checkpoint this directory no longer reads. Each is an entry the log /// once held that the disk cannot place, which is damage in the middle /// and gets damage's answer: refused rather than read around. Nothing is /// truncated or removed on this path. @@ -1074,7 +1042,7 @@ impl Wal { /// the mark's segment and the last. pub fn open_after(dir: &Path, from: Mark) -> Result<(Self, Replay), WalError> { create_dir(dir)?; - let on_disk = adopt(dir)?; + let on_disk = segments_in(dir)?; let (replay, discarded_from_total) = replay(dir, from, &on_disk)?; let (segment, segments, total) = match on_disk.last() { @@ -1463,56 +1431,14 @@ fn segments_in(dir: &Path) -> Result, WalError> { segments_under(LOG_FILE, dir).map_err(|err| WalError::io(dir, err)) } -/// Whether `dir` holds any byte of log: a segment, or a `pds.wal` from -/// before the log was segmented. +/// Whether `dir` holds any byte of log. /// /// What the guard against sweeping a restored blob tree asks before the log /// is opened. A file of zero bytes names nothing and counts as none. pub fn holds_bytes(dir: &Path) -> Result { - if std::fs::metadata(dir.join(LOG_FILE)).is_ok_and(|meta| meta.len() > 0) { - return Ok(true); - } Ok(segments_in(dir)?.iter().any(|(_, len)| *len > 0)) } -/// Takes a `pds.wal` from before the log was segmented in as segment zero, -/// and answers the segments on disk. -/// -/// A rename and a directory sync: the bytes are segment zero's already, and -/// a position a checkpoint took over that file is the same position in the -/// renamed one. One beside numbered segments was written by a build that -/// kept the log in one file after this one had segmented it, and holds -/// entries in no order this can place, so it is refused. -fn adopt(dir: &Path) -> Result, WalError> { - let legacy = dir.join(LOG_FILE); - let on_disk = segments_in(dir)?; - let len = match std::fs::metadata(&legacy) { - Ok(meta) => meta.len(), - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(on_disk), - Err(err) => return Err(WalError::io(&legacy, err)), - }; - if !on_disk.is_empty() { - return Err(WalError::Discontinuous { - path: legacy, - reason: format!( - "a {LOG_FILE} from before the log was segmented sits beside {} segments, and \ - its entries have no place in their order", - on_disk.len() - ), - }); - } - let first = dir.join(segment_name(0)); - std::fs::rename(&legacy, &first).map_err(|err| WalError::io(&legacy, err))?; - sync_dir(dir)?; - tracing::info!( - from = %legacy.display(), - to = %first.display(), - bytes = len, - "adopted the write-ahead log as its first segment" - ); - Ok(vec![(0, len)]) -} - /// Reads the log from `from` to its end, truncating a torn tail off the last /// segment, and answers how many bytes that cut. /// @@ -2561,59 +2487,6 @@ mod tests { marks } - /// A `pds.wal` from before the log was segmented is segment zero by - /// another name, and the open makes it that: the entries come back, and - /// the bytes are under the segment's name afterwards. - #[test] - fn a_log_from_before_segments_is_adopted_as_the_first_segment() { - let dir = scratch("adopt"); - { - let (wal, _) = Wal::open(&dir).expect("open"); - for name in ["older", "than", "segments"] { - wal.append(&claim(name), Durability::Sync).expect("append"); - } - } - std::fs::rename(dir.join(segment_name(0)), dir.join(LOG_FILE)).expect("un-segment it"); - assert!(holds_bytes(&dir).expect("readable")); - - let (wal, replay) = Wal::open(&dir).expect("reopen"); - assert_eq!(names_of(&replay), ["older", "than", "segments"]); - assert!(!dir.join(LOG_FILE).exists(), "the bare name is still there"); - assert_eq!(wal.path(), dir.join(segment_name(0))); - assert_eq!(wal.segments(), 1); - let _ = std::fs::remove_dir_all(&dir); - } - - /// A bare `pds.wal` beside numbered segments holds entries with no place - /// in their order, and is refused rather than adopted over them. - #[test] - fn a_log_from_before_segments_beside_segments_is_refused() { - let dir = scratch("adopt-beside"); - segmented(&dir, 2); - std::fs::write( - dir.join(LOG_FILE), - frame::encode(br#"{"op":"nameClaimed","name":"x"}"#), - ) - .expect("a bare log"); - let before = std::fs::read(dir.join(segment_name(1))).expect("readable"); - - let refusal = Wal::open(&dir).expect_err("two shapes of log in one directory opened"); - assert!( - matches!(refusal, WalError::Discontinuous { .. }), - "{refusal:?}" - ); - assert!( - dir.join(LOG_FILE).exists(), - "the refusal moved the bare log" - ); - assert_eq!( - std::fs::read(dir.join(segment_name(1))).expect("readable"), - before, - "the refusal touched a segment" - ); - let _ = std::fs::remove_dir_all(&dir); - } - /// A roll seals one segment and opens the next, and a replay from the /// origin walks every segment in order. #[test] @@ -2999,10 +2872,6 @@ mod tests { Entry::AccountRemoved { did: did.to_string(), }, - Entry::AccountPinned { - did: did.to_string(), - pinned: true, - }, Entry::AccountRetained { did: did.to_string(), retention: crate::kind::Retention::Forever, @@ -3033,12 +2902,6 @@ mod tests { did: did.to_string(), parent: "did:web:host.agents.localhost".to_owned(), }, - Entry::RecordPut { - did: did.to_string(), - collection: "com.example.thing".to_owned(), - rkey: "self".to_owned(), - record: serde_json::json!({}), - }, Entry::RecordRemoved { did: did.to_string(), collection: "com.example.thing".to_owned(), diff --git a/crates/didbot-pds/tests/durability.rs b/crates/didbot-pds/tests/durability.rs index fb31c0dd..aecf77ab 100644 --- a/crates/didbot-pds/tests/durability.rs +++ b/crates/didbot-pds/tests/durability.rs @@ -53,6 +53,20 @@ fn zone() -> Zone { Zone::delegated("localhost", ZONE_HOST).expect("test zone should be constructible") } +/// Writes this binary's layout stamp into a directory built by hand. +/// +/// A directory that holds state and no stamp is refused, so a test that +/// assembles the bytes itself has to say who wrote them. +fn stamp(dir: &Path) { + let rendered = serde_json::to_string(&didbot_pds::layout::Stamp { + layout: didbot_pds::layout::LAYOUT, + shape: didbot_pds::layout::SHAPE, + names: didbot_pds::layout::names(), + }) + .expect("a stamp serializes"); + std::fs::write(dir.join(didbot_pds::layout::STAMP_FILE), rendered).expect("the stamp writes"); +} + fn request(account_id: &str) -> ProvisionRequest { ProvisionRequest::new(account_id, None) } @@ -1610,6 +1624,7 @@ fn replay_parses_what_it_reads_rather_than_trusting_it() { bytes.extend_from_slice(&[0u8; 4]); bytes.extend_from_slice(&checked); std::fs::write(dir.join(didbot_pds::wal::segment_name(0)), &bytes).expect("write"); + stamp(&dir); let durable = Durable::open(&dir, Duration::days(30)).expect("open"); assert!(durable.accounts().list().is_empty()); @@ -2730,19 +2745,21 @@ fn frame(payload: &[u8]) -> Vec { out } -/// One entry of every shape a log written before this binary holds, as bytes +/// One entry of every shape a log holds, written out as the bytes on disk /// rather than as constructed values. /// /// Constructed values would follow the types wherever they went and agree -/// with themselves afterwards. These are the JSON a build wrote, so a change -/// that moved a field, retagged a variant or let one variant's bytes route -/// into another's is a failure here rather than a directory that opens and -/// means something else. +/// with themselves afterwards. These are the JSON, so a change that moved a +/// field, retagged a variant or let one variant's bytes route into another's +/// is a failure here rather than a directory that opens and means something +/// else. const WRITTEN_ENTRIES: &[(&str, &str)] = &[ ( "accountInserted", r#"{"op":"accountInserted","account":{"did":"did:web:shearwater.agents.localhost", - "accountId":"shearwater","handle":null,"createdAt":"2026-01-01T00:00:00Z"}, + "accountId":"shearwater","handle":null,"createdAt":"2026-01-01T00:00:00Z", + "kind":"agent","nameProvenance":"issued","retention":"deployment", + "state":"active"}, "key":"0101010101010101010101010101010101010101010101010101010101010101"}"#, ), ( @@ -2750,10 +2767,6 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ r#"{"op":"accountParented","did":"did:web:a.agents.localhost", "parent":"did:web:host.agents.localhost"}"#, ), - ( - "accountPinned", - r#"{"op":"accountPinned","did":"did:web:a.agents.localhost","pinned":true}"#, - ), ( "accountRemoved", r#"{"op":"accountRemoved","did":"did:web:a.agents.localhost"}"#, @@ -2822,11 +2835,6 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ "access_expires_at":"2026-01-01T00:10:00Z", "refresh_expires_at":"2026-01-15T00:05:00Z"}"#, ), - ( - "recordPut", - r#"{"op":"recordPut","did":"did:web:a.agents.localhost","collection":"com.example.thing", - "rkey":"3lb","record":{"text":"weftling"}}"#, - ), ( "recordRemoved", r#"{"op":"recordRemoved","did":"did:web:a.agents.localhost", @@ -2847,16 +2855,14 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ ), ]; -/// Every entry a build before this one wrote is still read as itself. +/// Every entry reads back as the variant that wrote it. /// -/// The programme this pull opens moves a store at a time, so a data -/// directory holds a mixture of what earlier builds wrote and what later -/// ones do. That only works while the earlier half keeps decoding, and /// `Entry` is a tagged enum over serde, which is forgiving in exactly the -/// direction that hides the failure: bytes that route into the wrong variant -/// parse, and the deployment comes up looking healthy. +/// direction that hides a failure: bytes that route into the wrong variant +/// parse, and the deployment comes up looking healthy. This pins the bytes +/// each variant is on disk so that routing is a failing test. #[test] -fn every_entry_a_previous_build_wrote_still_decodes_as_itself() { +fn every_entry_reads_back_as_the_variant_that_wrote_it() { // The corpus is the whole of what came before, so a variant added // without one is caught here rather than by nobody. let covered: Vec<&str> = WRITTEN_ENTRIES.iter().map(|(op, _)| *op).collect(); @@ -2873,7 +2879,7 @@ fn every_entry_a_previous_build_wrote_still_decodes_as_itself() { for (op, raw) in WRITTEN_ENTRIES { let entry: didbot_pds::wal::Entry = serde_json::from_str(raw) - .unwrap_or_else(|error| panic!("a `{op}` a previous build wrote: {error}")); + .unwrap_or_else(|error| panic!("the bytes a `{op}` is written as: {error}")); let back = serde_json::to_value(&entry).expect("an entry serializes"); assert_eq!( back["op"].as_str(), diff --git a/crates/didbot-pds/tests/ledger.rs b/crates/didbot-pds/tests/ledger.rs index 6d2abe3a..70f275e8 100644 --- a/crates/didbot-pds/tests/ledger.rs +++ b/crates/didbot-pds/tests/ledger.rs @@ -116,13 +116,11 @@ fn provisioning_opens_a_ledger_naming_the_backend_that_admitted_it() { account_id, backend, assurance, - node_id, .. } => { assert_eq!(account_id, "kestrel"); assert_eq!(backend, "server"); assert_eq!(assurance, didbot_pds::CREATOR_ASSURANCE); - assert_eq!(node_id, &None); } other => panic!("the first entry must be a provisioning: {other:?}"), } diff --git a/crates/didbot-pds/tests/upgrade.rs b/crates/didbot-pds/tests/upgrade.rs index 1aa915aa..76a5c4bc 100644 --- a/crates/didbot-pds/tests/upgrade.rs +++ b/crates/didbot-pds/tests/upgrade.rs @@ -268,78 +268,81 @@ fn a_stamp_from_another_binary_is_refused_before_the_directory_is_touched() { let _ = std::fs::remove_dir_all(&dir); } -/// The upgrade a deployment actually performs. +/// A stamp missing its names half is refused, and the refusal costs nothing. /// -/// Every data directory written before the names half landed carries -/// `{layout, shape}` and no `names`. Refusing those makes the first upgrade -/// of a running deployment a boot failure with no remedy but deleting the -/// data, so an agreeing `shape` backfills the missing half instead. -/// -/// The assertions that matter are not that the open returned `Ok`. They are -/// that the blob is still fetchable afterwards — a backfill that let the -/// sweep run against names it had guessed wrong would return `Ok` too — and -/// that the stamp left behind is a whole one, so the next boot is ordinary. +/// The shape agreeing says the log would be read correctly; it says nothing +/// about where the blobs are, and `FileBlobStore::sweep` deletes any file +/// whose name the replayed index did not expect. So the directory is refused +/// — and the assertions that matter are the same two the test above makes: +/// the log is byte-for-byte what it was, and the blobs are still there. #[test] -fn a_stamp_from_before_the_names_half_is_adopted_rather_than_refused() { - let dir = scratch("names-backfill"); +fn a_stamp_missing_the_names_half_is_refused_before_the_directory_is_touched() { + let dir = scratch("names-half"); let (did, reference) = populate(&dir); let account = dir.join(BLOB_DIR).join(didbot_pds::blobs::did_path(&did)); let blob = account.join(&reference.cid); assert!(blob.is_file(), "the fixture should have written a blob"); - // A file the replayed index does not name. It is here so that "the blob - // survived" means something: an adopted open that never reached the - // sweep would keep both, and this asserts the sweep really ran. let stranger = account.join("bafkreibvjvcv745gucpxq53k5nyfk3ehwyoypp2xrjvzt3xn5x5rjcxvpa"); std::fs::write(&stranger, "unnamed").expect("a stranger's blob"); - // Byte-for-byte the stamp a build from before the names half wrote: - // this binary's real `SHAPE`, and no `names` key at all. + let log = live_segment(&dir); + let before = std::fs::read(&log).expect("the log is readable"); + + // This binary's real `SHAPE`, and no `names` key at all. let stamp = dir.join(STAMP_FILE); - std::fs::write( - &stamp, - serde_json::to_string(&json!({"layout": LAYOUT - 1, "shape": SHAPE})) - .expect("a stamp serializes"), - ) - .expect("the stamp should write"); + let raw = serde_json::to_string(&json!({"layout": LAYOUT - 1, "shape": SHAPE})) + .expect("a stamp serializes"); + std::fs::write(&stamp, &raw).expect("the stamp should write"); - let durable = Durable::open(&dir, Duration::days(30)).expect("an agreeing shape is adopted"); + let err = Durable::open(&dir, Duration::days(30)).expect_err("half a stamp is refused"); assert!( - durable.blobs().fetch(&did, &reference.cid).is_ok(), - "the adopted directory's blob is still there" + matches!(err, WalError::Layout(LayoutError::Legacy { .. })), + "{err}" + ); + assert_eq!( + std::fs::read(&log).expect("the log is still readable"), + before, + "a refused open must not have touched the log" ); - drop(durable); - assert!(blob.is_file(), "the adopted open must not sweep the blobs"); assert!( - !stranger.exists(), - "the adopted open is a real open, so the sweep it runs is a real sweep" + stranger.is_file() && blob.is_file(), + "a refused open must not have swept the blobs" ); - - let written: Stamp = - serde_json::from_str(&std::fs::read_to_string(&stamp).expect("the stamp is readable")) - .expect("a whole stamp was written"); assert_eq!( - written, - Stamp { - layout: LAYOUT, - shape: SHAPE, - names: names(), - } + std::fs::read_to_string(&stamp).expect("the stamp is readable"), + raw, + "a refused open must not have rewritten the stamp" ); - // And a pre-names stamp whose shape does *not* agree stays refused: the - // shape is the only evidence the names half has, so without it there is - // nothing to back a backfill. + let _ = std::fs::remove_dir_all(&dir); +} + +/// A directory that holds state and carries no stamp is refused too. +/// +/// A restore that lost `pds.layout` and a directory an older build wrote +/// look the same from here, and stamping it would be this binary asserting a +/// shape over bytes nothing on disk describes. +#[test] +fn an_unstamped_directory_holding_state_is_refused() { + let dir = scratch("unstamped"); + let (did, reference) = populate(&dir); + let blob = dir + .join(BLOB_DIR) + .join(didbot_pds::blobs::did_path(&did)) + .join(&reference.cid); let log = live_segment(&dir); let before = std::fs::read(&log).expect("the log is readable"); - let foreign = serde_json::to_string(&json!({"layout": LAYOUT - 1, "shape": SHAPE ^ 0x5eed})) - .expect("a stamp serializes"); - std::fs::write(&stamp, &foreign).expect("the stamp should write"); + std::fs::remove_file(dir.join(STAMP_FILE)).expect("the stamp goes"); - let err = Durable::open(&dir, Duration::days(30)).expect_err("a foreign shape is refused"); + let err = Durable::open(&dir, Duration::days(30)).expect_err("an unstamped directory refuses"); assert!( - matches!(err, WalError::Layout(LayoutError::Legacy { .. })), + matches!(err, WalError::Layout(LayoutError::Unstamped { .. })), "{err}" ); + assert!( + !dir.join(STAMP_FILE).exists(), + "the refusal must not stamp the directory" + ); assert_eq!( std::fs::read(&log).expect("the log is still readable"), before, @@ -349,11 +352,8 @@ fn a_stamp_from_before_the_names_half_is_adopted_rather_than_refused() { blob.is_file(), "a refused open must not have swept the blobs" ); - assert_eq!( - std::fs::read_to_string(&stamp).expect("the stamp is readable"), - foreign, - "a refused open must not have rewritten the stamp" - ); + let rendered = err.to_string(); + assert!(rendered.contains(STAMP_FILE), "{rendered}"); let _ = std::fs::remove_dir_all(&dir); } diff --git a/crates/didbot-serve/src/tests/mod.rs b/crates/didbot-serve/src/tests/mod.rs index 5c9f5ccd..1f114933 100644 --- a/crates/didbot-serve/src/tests/mod.rs +++ b/crates/didbot-serve/src/tests/mod.rs @@ -1002,7 +1002,6 @@ impl Registry for FakeRegistry { account_id: account.account_id.clone(), backend: "server".to_owned(), assurance: didbot_pds::CREATOR_ASSURANCE.to_owned(), - node_id: None, parent: None, }, }], diff --git a/crates/didbot/tests/scenarios/restart.rs b/crates/didbot/tests/scenarios/restart.rs index 906d802e..61424caf 100644 --- a/crates/didbot/tests/scenarios/restart.rs +++ b/crates/didbot/tests/scenarios/restart.rs @@ -495,7 +495,7 @@ async fn a_data_directory_from_layout_16_is_refused_by_name() { let mut stamp: Value = serde_json::from_str(&std::fs::read_to_string(&stamp_path).expect("a stamp")) .expect("json"); - assert_eq!(stamp["layout"], 17, "{stamp}"); + assert_eq!(stamp["layout"], 18, "{stamp}"); // What the build before this stack wrote: its own number, and a shape // that is not this one's (the `agentToken*` entries were renamed). stamp["layout"] = json!(16); @@ -507,10 +507,7 @@ async fn a_data_directory_from_layout_16_is_refused_by_name() { eprintln!("layout: exit {code}\n{logged}"); assert_ne!(code, 0); assert!(logged.contains("written as version 16"), "{logged}"); - assert!( - logged.contains("check out the build that wrote it, or delete the directory"), - "{logged}" - ); + assert!(logged.contains("delete the directory"), "{logged}"); assert_eq!( std::fs::read(server.data_dir().join("pds.wal.000000")).expect("the log"), wal_before, diff --git a/docs/operations.md b/docs/operations.md index 8aabc895..8fa594e6 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -36,6 +36,7 @@ then restarts every five seconds logging the same message. |---|---|---| | `the data directory at … was written as version …` | The stamp disagrees with the binary | "Rolling back" | | `the data directory at … carries a stamp written before` this binary's check | Half the stamp has nothing to compare against | Run the build that wrote it, or delete the directory | +| `the data directory at … holds state and carries no pds.layout` | The directory says nothing about what wrote it | "Restoring" — bring `pds.layout` back with the rest of the volume | | `blob bytes are present and pds.wal names none of them` | Blob files with no log to name them | "Restoring" | | `entry did not parse` | Intact bytes this binary does not read as an entry | Run the build that wrote it. Do not delete frames by hand | | Another process holds the lock | A second writer against one directory | Find and stop it; never mount one data directory twice | @@ -43,15 +44,16 @@ then restarts every five seconds logging the same message. ## Rolling back -A data directory carrying no `pds.layout` stamp is adopted and stamped with the -opening binary's own layout, so the first run of a new binary restamps it and -the old binary then meets a mismatch on a directory it wrote itself. +A data directory carries a `pds.layout` stamp naming the layout that wrote it, +and a binary refuses a stamp it does not read. That runs one way: a new binary +restamps the directory, and the old one then meets a mismatch on a directory it +wrote itself. **The rollback is a restore.** Run the old image against a snapshot taken -before the upgrade: its stamp is the old layout, or absent, and either is -accepted. The cost is everything written since that snapshot — up to -twenty-four hours on the daily plan alone, minutes with an on-demand snapshot -taken just before the swap. +before the upgrade: its stamp is the old layout, which that image accepts. The +cost is everything written since that snapshot — up to twenty-four hours on the +daily plan alone, minutes with an on-demand snapshot taken just before the +swap. ## Backups @@ -67,6 +69,8 @@ Snapshot the volume; do not `rsync` the directory. - Create a volume from a recovery point in the `didbot-pds` vault, in the target instance's availability zone. +- Copy the whole directory, `pds.layout` included. A volume that came back + without it is refused: the directory then says nothing about what wrote it. - Attach it to a **fresh** instance, never to the running one, and mount it at `/data`, where that instance's boot script chowns `/data/pds` to uid 10001. The image runs as that uid and chmods the data directory to `0700` on every diff --git a/docs/running-locally.md b/docs/running-locally.md index 6334f1bd..d3b4aaaf 100644 --- a/docs/running-locally.md +++ b/docs/running-locally.md @@ -176,8 +176,8 @@ build that wrote it, or delete the directory and start again Nothing converts one layout to another. In development the answer is to delete the directory, which costs a factory reset, and it is safe to delete whenever -you want one anyway. A directory from before stamping existed is adopted and -stamped rather than refused. +you want one anyway. A directory that holds something and carries no +`pds.layout` is refused too: nothing there says what wrote it. A missing log is refused too, but only when the directory is not really empty. Startup reconciles `blobs/` against what the log names, and a file the log does -- 2.51.2