Skip to content

Commit b837b39

Browse files
authored
Give hot ops shared attributes, and close an interaction with a flagged Meta op (#4753)
* Give hot ops shared attributes, and close an interaction with a flagged Meta op * Review fixes * Cleanup
1 parent c7e9d79 commit b837b39

10 files changed

Lines changed: 191 additions & 9 deletions

File tree

‎document/format/tests/open_create.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ fn apply_hot_op_persists_to_hot_log_and_survives_reopen() {
123123
},
124124
timestamp: TimeStamp { counter: 1, peer: PeerId(5) },
125125
sequence: HotSequence(1),
126+
attributes: Default::default(),
126127
};
127128
gdd.apply_hot_op(hot_op).unwrap_or_else(|error| panic!("apply_hot_op failed: {error:?}"));
128129

@@ -148,6 +149,7 @@ fn retire_moves_eligible_hot_ops_to_history_and_keeps_rest() {
148149
},
149150
timestamp: TimeStamp { counter: 1, peer: PeerId(5) },
150151
sequence: HotSequence(1),
152+
attributes: Default::default(),
151153
};
152154
let late = HotOp {
153155
op: RegistryDelta::AddNetwork {
@@ -156,6 +158,7 @@ fn retire_moves_eligible_hot_ops_to_history_and_keeps_rest() {
156158
},
157159
timestamp: TimeStamp { counter: 10, peer: PeerId(5) },
158160
sequence: HotSequence(2),
161+
attributes: Default::default(),
159162
};
160163
gdd.apply_hot_op(early).unwrap();
161164
gdd.apply_hot_op(late).unwrap();
@@ -194,6 +197,7 @@ fn last_broadcast_rev_persists_across_reopen() {
194197
},
195198
timestamp: TimeStamp { counter: 1, peer: PeerId(5) },
196199
sequence: HotSequence(1),
200+
attributes: Default::default(),
197201
};
198202
gdd.apply_hot_op(op).unwrap();
199203
let retired = gdd.retire(TimeStamp { counter: 1, peer: PeerId(5) }).unwrap_or_else(|error| panic!("retire failed: {error:?}"));
@@ -632,6 +636,7 @@ fn persist_path_writes_at_manifest_declared_codec_paths() {
632636
},
633637
timestamp: TimeStamp { counter: 1, peer: PeerId(5) },
634638
sequence: HotSequence(1),
639+
attributes: Default::default(),
635640
};
636641
gdd.apply_hot_op(hot_op).unwrap_or_else(|error| panic!("apply_hot_op failed: {error:?}"));
637642

‎document/format/tests/staging_and_reopen.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ fn a_reopen_keeps_unretired_hot_ops_out_of_the_retired_snapshot() {
5454
},
5555
timestamp: TimeStamp { counter, peer: PeerId(22) },
5656
sequence: HotSequence(counter),
57+
attributes: Default::default(),
5758
};
5859
futures::executor::block_on(async {
5960
let mut gdd = GddV1::create_in(AnyContainer::Memory(MemoryBackend::new()), GddV1Layout, PeerId(22), 1, "ed".into(), "std".into()).unwrap();

‎document/graph-storage/src/attributes/attr.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ pub mod network {
6767
}
6868

6969
pub mod delta {
70-
/// Marks the last delta of a user interaction, so the undo cursor steps per-interaction, not per-delta.
70+
/// Marks the last delta of a user interaction, so the undo cursor steps per-interaction, not per-delta. Also set on
71+
/// the last hot op of an interaction.
7172
pub const INTERACTION_END: &str = "interaction_end";
7273
}

‎document/graph-storage/src/crdt.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ pub enum RegistryDelta {
211211
Merge {
212212
extra_parents: Vec<Rev>,
213213
},
214+
Meta,
214215
// Allow for future delta types without a model change
215216
Other(Value),
216217
}

‎document/graph-storage/src/document.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ impl Document {
350350
registry.attributes.apply_delta(delta, timestamp);
351351
}
352352
// Merge is a structural sync point only; it mutates no registry state.
353-
RegistryDelta::Merge { .. } | RegistryDelta::Other(_) => {}
353+
RegistryDelta::Merge { .. } | RegistryDelta::Meta | RegistryDelta::Other(_) => {}
354354
}
355355
Ok(())
356356
}

‎document/graph-storage/src/prior.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ pub(crate) fn capture(registry: &Registry, op: &RegistryDelta) -> Vec<Prior> {
184184
key: delta.key.clone(),
185185
previous: registry.attributes.get(&delta.key).cloned(),
186186
}),
187-
RegistryDelta::Merge { .. } | RegistryDelta::Other(_) => {}
187+
RegistryDelta::Merge { .. } | RegistryDelta::Meta | RegistryDelta::Other(_) => {}
188188
}
189189
priors
190190
}

‎document/graph-storage/src/session.rs‎

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use crate::NodeMetadataSource;
33
#[cfg(any(feature = "conversion", test))]
44
use crate::from_runtime;
55
use crate::{
6-
ApplyMode, Delta, Document, History, Implementation, LamportClock, NetworkId, NodeId, PeerId, Registry, RegistryDelta, RegistryTarget, ResourceEntry, Rev, TimeStamp, UserId, Value, to_value,
6+
ApplyMode, Attributes, Delta, Document, History, Implementation, LamportClock, NetworkId, NodeId, PeerId, Registry, RegistryDelta, RegistryTarget, ResourceEntry, Rev, TimeStamp, UserId, Value,
7+
to_value,
78
};
89
use graphene_resource::{ResourceHash, ResourceId};
910
use serde::{Deserialize, Serialize};
@@ -146,15 +147,19 @@ impl Session {
146147
/// `PeerId → UserId` mapping is established (and, under causal delivery, observed by other peers)
147148
/// before any of its edits. A no-op batch doesn't register, since registration rides a real edit.
148149
pub(crate) fn stage_ops(&mut self, ops: impl IntoIterator<Item = RegistryDelta>) -> Result<Vec<HotOp>, CrdtError> {
150+
self.stage_ops_with(ops.into_iter().map(|op| (op, Attributes::new())))
151+
}
152+
153+
fn stage_ops_with(&mut self, ops: impl IntoIterator<Item = (RegistryDelta, Attributes)>) -> Result<Vec<HotOp>, CrdtError> {
149154
// A batch is staged whole or not at all.
150-
let mut pending: Vec<RegistryDelta> = ops.into_iter().collect();
155+
let mut pending: Vec<(RegistryDelta, Attributes)> = ops.into_iter().collect();
151156
if pending.is_empty() {
152157
return Ok(Vec::new());
153158
}
154159

155160
let (peer, user) = (self.document.peer, self.document.user);
156161
if self.document.working_registry.peer_users.get(&peer).map(|registration| registration.user) != Some(user) {
157-
pending.insert(0, RegistryDelta::RegisterPeer { peer, user });
162+
pending.insert(0, (RegistryDelta::RegisterPeer { peer, user }, Attributes::new()));
158163
}
159164

160165
// A reused sequence would fall under the retired marks, so a batch running past the end is refused.
@@ -164,12 +169,13 @@ impl Session {
164169

165170
let sequence_before = self.document.last_hot_sequence;
166171
let mut staged = Vec::with_capacity(pending.len());
167-
for op in pending {
172+
for (op, attributes) in pending {
168173
let sequence = HotSequence(self.document.last_hot_sequence.0 + 1);
169174
let hot_op = HotOp {
170175
op,
171176
timestamp: self.document.clock.tick(),
172177
sequence,
178+
attributes,
173179
};
174180
if let Err(error) = self.document.stage_hot_op(hot_op.clone(), ApplyMode::Strict) {
175181
// The run continues where it was, so no sequence goes unused.
@@ -344,22 +350,79 @@ impl Session {
344350
self.document.history.len()
345351
}
346352

353+
/// Closes this peer's open interaction with a flagged [`RegistryDelta::Meta`] op. Returns the staged ops, a
354+
/// re-registration possibly before the marker; empty when nothing is open.
355+
pub fn end_interaction(&mut self) -> Result<Vec<HotOp>, CrdtError> {
356+
let peer = self.document.peer;
357+
let open = self
358+
.document
359+
.hot_log
360+
.iter()
361+
.rev()
362+
.find(|hot_op| hot_op.timestamp.peer == peer)
363+
.is_some_and(|hot_op| !hot_op.ends_interaction());
364+
if !open {
365+
return Ok(Vec::new());
366+
}
367+
let mut closing = Attributes::new();
368+
closing.set(crate::attr::delta::INTERACTION_END, Value::Bool(true), TimeStamp::ORIGIN);
369+
self.stage_ops_with([(RegistryDelta::Meta, closing)])
370+
}
371+
372+
/// Every author's closed interactions in the hot log, the earliest closed first.
373+
pub fn closed_interactions(&self) -> Vec<ClosedInteraction> {
374+
let mut by_author: HashMap<PeerId, Vec<&HotOp>> = HashMap::new();
375+
for hot_op in &self.document.hot_log {
376+
by_author.entry(hot_op.timestamp.peer).or_default().push(hot_op);
377+
}
378+
379+
let mut closed = Vec::new();
380+
for (author, mut ops) in by_author {
381+
ops.sort_by_key(|hot_op| hot_op.sequence);
382+
let mut expected = self.document.retired.retired_up_to.get(&author).copied().unwrap_or(HotSequence::NONE).next();
383+
let mut current = Vec::new();
384+
let mut contiguous = true;
385+
for hot_op in ops {
386+
// Retired past a gap: not in the log, but not missing either.
387+
while expected < hot_op.sequence && self.document.retired.covers(HotOpId { peer: author, sequence: expected }) {
388+
expected = expected.next();
389+
}
390+
contiguous &= hot_op.sequence == expected;
391+
expected = hot_op.sequence.next();
392+
current.push(hot_op.id());
393+
if hot_op.ends_interaction() {
394+
closed.push(ClosedInteraction {
395+
author,
396+
ops: std::mem::take(&mut current),
397+
closed_at: hot_op.timestamp,
398+
contiguous,
399+
});
400+
}
401+
}
402+
}
403+
closed.sort_by_key(|interaction| interaction.closed_at);
404+
closed
405+
}
406+
347407
/// The hot ops stamped at or before `up_to`, which [`retire`](Self::retire) drains.
348408
pub fn hot_ops_up_to(&self, up_to: TimeStamp) -> Vec<HotOpId> {
349409
self.document.hot_log.iter().filter(|hot_op| hot_op.timestamp <= up_to).map(HotOp::id).collect()
350410
}
351411

352412
/// Promote the given hot ops into retired deltas in hot-log order, each keeping its authored stamp.
353413
///
354-
/// One retired delta per hot op.
414+
/// One retired delta per hot op, except a `Meta` op, which commits nothing.
355415
pub fn retire_hot_ops(&mut self, ids: &[HotOpId]) -> Result<Vec<Rev>, CrdtError> {
356416
let wanted: HashSet<HotOpId> = ids.iter().copied().collect();
357417
let (drained, kept): (Vec<HotOp>, Vec<HotOp>) = self.document.hot_log.drain(..).partition(|hot_op| wanted.contains(&hot_op.id()));
358418
self.document.hot_log = kept;
359419
self.document.resync_hot_timestamps();
360420
self.document.retired.extend(drained.iter().map(HotOp::id));
361421

362-
let ops = drained.into_iter().map(|hot_op| (hot_op.op, Some(hot_op.timestamp)));
422+
let ops = drained
423+
.into_iter()
424+
.filter(|hot_op| !matches!(hot_op.op, RegistryDelta::Meta))
425+
.map(|hot_op| (hot_op.op, Some(hot_op.timestamp)));
363426
self.commit_ops_authored_at(ops, true)
364427
}
365428

@@ -627,9 +690,16 @@ pub struct HotOp {
627690
pub timestamp: TimeStamp,
628691
/// Position in its author's run. See [`HotSequence`].
629692
pub sequence: HotSequence,
693+
/// Shared with every peer, unlike `Delta::attributes`. Dropped at retirement.
694+
#[serde(default)]
695+
pub attributes: Attributes,
630696
}
631697

632698
impl HotOp {
699+
pub fn ends_interaction(&self) -> bool {
700+
self.attributes.get_or(crate::attr::delta::INTERACTION_END, false)
701+
}
702+
633703
/// Identifies the op in the retired marks, which track a contiguous prefix per author.
634704
pub fn id(&self) -> HotOpId {
635705
HotOpId {
@@ -639,6 +709,17 @@ impl HotOp {
639709
}
640710
}
641711

712+
#[derive(Clone, Debug, PartialEq, Eq)]
713+
pub struct ClosedInteraction {
714+
pub author: PeerId,
715+
/// In the author's order, the marker last.
716+
pub ops: Vec<HotOpId>,
717+
/// The marker's stamp.
718+
pub closed_at: TimeStamp,
719+
/// No gap in the author's sequence since its retired frontier.
720+
pub contiguous: bool,
721+
}
722+
642723
/// Position in one author's run of hot ops, counting from 1 with no gaps. Kept apart from the Lamport
643724
/// counter, which skips on observing a higher remote timestamp and so cannot bound a contiguous prefix.
644725
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize, Deserialize)]

‎document/graph-storage/src/tests/bookkeeping.rs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ fn retirement_takes_the_ops_under_the_cutoff_in_any_applied_order() {
2525
op,
2626
timestamp: crate::TimeStamp { counter, peer: PeerId(peer) },
2727
sequence: crate::HotSequence(1),
28+
attributes: Default::default(),
2829
};
2930
let (late, own) = (hot(add_network(9), 50, 2), hot(set_attribute("x", 1), 20, 3));
3031
host.replay_hot_op(late.clone()).expect("the late-stamped op lands first");
@@ -83,6 +84,7 @@ fn a_reopened_session_drops_a_late_copy_of_a_retired_op() {
8384
op: set_attribute("k", 1),
8485
timestamp: crate::TimeStamp { counter: 3, peer: PeerId(2) },
8586
sequence: crate::HotSequence(1),
87+
attributes: Default::default(),
8688
};
8789
let mut host = Session::with_peer(PeerId(1));
8890
host.replay_hot_op(op.clone()).unwrap();
@@ -165,6 +167,7 @@ fn a_retired_late_copy_still_advances_the_sequence_and_the_clock() {
165167
op: set_attribute("k", 1),
166168
timestamp: crate::TimeStamp { counter: 50, peer: PeerId(1) },
167169
sequence: crate::HotSequence(7),
170+
attributes: Default::default(),
168171
};
169172
let mut marks = crate::RetiredMarks::default();
170173
marks.extend([own.id()]);
@@ -176,3 +179,88 @@ fn a_retired_late_copy_still_advances_the_sequence_and_the_clock() {
176179
assert!(next.sequence > own.sequence, "a fresh sequence");
177180
assert!(next.timestamp > own.timestamp, "a later stamp");
178181
}
182+
183+
#[test]
184+
fn only_closed_interactions_are_listed_and_the_marker_commits_nothing() {
185+
let mut host = Session::with_peer(PeerId(1));
186+
let guest_op = |counter: u64, sequence: u64, op: RegistryDelta| crate::HotOp {
187+
op,
188+
timestamp: crate::TimeStamp { counter, peer: PeerId(2) },
189+
sequence: crate::HotSequence(sequence),
190+
attributes: Default::default(),
191+
};
192+
193+
assert!(host.end_interaction().expect("nothing to close").is_empty(), "no op of this peer is open");
194+
195+
// The guest's open interaction is stamped before everything the host does.
196+
host.replay_hot_op(guest_op(10, 1, add_network(9))).expect("guest op");
197+
host.replay_hot_op(guest_op(11, 2, set_attribute("guest", 1))).expect("guest op");
198+
199+
host.stage_ops([set_attribute("host", 1)]).expect("host op");
200+
let marker = host.end_interaction().expect("close").pop().expect("the host's interaction was open");
201+
assert!(matches!(marker.op, RegistryDelta::Meta) && marker.ends_interaction());
202+
assert!(host.end_interaction().expect("nothing more to close").is_empty(), "closing twice stages nothing");
203+
204+
let closed = host.closed_interactions();
205+
assert_eq!(closed.len(), 1, "the guest's open interaction is not listed: {closed:?}");
206+
assert_eq!(closed[0].author, PeerId(1));
207+
assert!(closed[0].contiguous);
208+
assert_eq!(closed[0].ops.len(), 3, "RegisterPeer, the attribute write and the marker");
209+
210+
let revs = host.retire_hot_ops(&closed[0].ops).expect("retire");
211+
assert_eq!(revs.len(), 2, "the marker commits no delta");
212+
assert_eq!(host.hot_log().len(), 2, "the guest's ops stay hot");
213+
214+
host.replay_hot_op(closing(guest_op(12, 3, RegistryDelta::Meta))).expect("guest marker");
215+
let closed = host.closed_interactions();
216+
assert_eq!(closed.len(), 1);
217+
assert_eq!(closed[0].author, PeerId(2));
218+
assert_eq!(closed[0].ops.len(), 3);
219+
}
220+
221+
#[test]
222+
fn an_interaction_with_a_gap_is_not_contiguous_until_the_gap_fills() {
223+
let mut host = Session::with_peer(PeerId(1));
224+
let guest_op = |counter: u64, sequence: u64, op: RegistryDelta| crate::HotOp {
225+
op,
226+
timestamp: crate::TimeStamp { counter, peer: PeerId(2) },
227+
sequence: crate::HotSequence(sequence),
228+
attributes: Default::default(),
229+
};
230+
host.replay_hot_op(guest_op(10, 1, add_network(9))).expect("guest op");
231+
host.replay_hot_op(closing(guest_op(12, 3, RegistryDelta::Meta))).expect("guest marker, op 2 still in flight");
232+
host.replay_hot_op(closing(guest_op(13, 4, set_attribute("guest", 2)))).expect("a later interaction");
233+
234+
let closed = host.closed_interactions();
235+
assert_eq!(closed.len(), 2);
236+
assert!(!closed[0].contiguous);
237+
assert!(!closed[1].contiguous, "the gap is behind the later interaction too");
238+
239+
host.replay_hot_op(guest_op(11, 2, set_attribute("guest", 1))).expect("the late op");
240+
let closed = host.closed_interactions();
241+
assert!(closed.iter().all(|interaction| interaction.contiguous));
242+
assert_eq!(closed[0].ops.len(), 3);
243+
}
244+
245+
#[test]
246+
fn a_flagged_write_closes_its_interaction() {
247+
let mut host = Session::with_peer(PeerId(1));
248+
let guest_op = |counter: u64, sequence: u64, op: RegistryDelta| crate::HotOp {
249+
op,
250+
timestamp: crate::TimeStamp { counter, peer: PeerId(2) },
251+
sequence: crate::HotSequence(sequence),
252+
attributes: Default::default(),
253+
};
254+
host.replay_hot_op(guest_op(10, 1, add_network(9))).expect("guest op");
255+
assert!(host.closed_interactions().is_empty(), "the interaction is open");
256+
257+
host.replay_hot_op(closing(guest_op(11, 2, set_attribute("guest", 1)))).expect("flagged write");
258+
let closed = host.closed_interactions();
259+
assert_eq!(closed.len(), 1);
260+
assert_eq!(closed[0].ops.len(), 2, "both writes, and no marker of its own");
261+
}
262+
263+
fn closing(mut hot_op: crate::HotOp) -> crate::HotOp {
264+
hot_op.attributes.set(crate::attr::delta::INTERACTION_END, Value::Bool(true), crate::TimeStamp::ORIGIN);
265+
hot_op
266+
}

‎document/graph-storage/src/tests/crdt.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ fn apply_hot_op_advances_clock_past_observed_timestamp() {
4141
op: remove_node_op(NodeId(99)),
4242
timestamp: observed,
4343
sequence: crate::HotSequence(1),
44+
attributes: Default::default(),
4445
};
4546

4647
document
@@ -978,6 +979,7 @@ fn hot_op(op: RegistryDelta, counter: u64, peer: u64, sequence: u64) -> HotOp {
978979
op,
979980
timestamp: ts(counter, peer),
980981
sequence,
982+
attributes: Default::default(),
981983
}
982984
}
983985

‎node-graph/rfcs/document-format.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ pub enum RegistryDelta {
123123
AddResource { id: ResourceId, entry: ResourceEntry }, // whole-entry; reverse of RemoveResource
124124
RemoveResource { id: ResourceId, snapshot: ResourceEntry }, // snapshot for O(1) reverse
125125
Merge { extra_parents: Vec<Rev> }, // joins divergent tips; registry no-op
126+
Meta,
126127
Other(Value), // forward-compatible escape hatch
127128
}
128129

@@ -157,6 +158,8 @@ One timestamp per `Delta` applies to every LWW-eligible write inside its `kind`.
157158

158159
`Delta.attributes` is a type-erased annotation bucket (the same shape as the registry's attribute buckets) for mutable, local-only labels: the `interaction_end` marker that bounds undo units, and later commit messages. It is **excluded from `id`** so that annotating a delta never changes its content-addressed identity. An inline write sets it before the delta's history frame is persisted, while a later relabel rewrites that frame.
159160

161+
`HotOp.attributes` is the same bucket on a hot op, but shared: it travels with the op to every peer and is left behind at retirement. The same `interaction_end` key marks the last op of its author's interaction, so the retirer takes the author's ops through it as one unit. When the boundary comes after the last write was sent, a data-less `RegistryDelta::Meta` op carries the flag; retirement drops `Meta` ops. The retirer marks the unit's last retired delta with `Delta.attributes`' own `interaction_end`, so the boundary survives without the hot op.
162+
160163
## History as a tree
161164

162165
History is a multi-parent DAG. Branching is implicit: every concurrent or out-of-sync edit creates a branch by virtue of sharing a parent with another delta. Divergent tips are rejoined by an explicit `Merge` delta listing the joined tips, which is a registry no-op (it only collapses the tips so `head` stays a single `Rev`).

0 commit comments

Comments
 (0)