| author | |
| committer | |
| log | cf24fa511d61da2e5d87246aad01eccced587479 |
| tree | 28805bbd1c5c742c1b11ecb510913ab377bc0340 |
| parent | 860f9341c754f84c8ce5d06a103d2d1fd39fefdb |
| signature | Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU |
new_id drew a fresh GUID per object, and a stored object carries an id-table
entry per distinct GUID it references in every revision that rewrites it.
Identities now share a GUID per 255 allocations, as OneNote numbers objects
within a session, so content authored in one session costs its containers one
table entry. (The restart soak that exposed the table cost starts a process
per two edits and is unchanged by this: one GUID per launch either way.)
The mixed-client soak verifier predated save coalescing and rejected intents
shared by consecutive operations; it now accounts a coalesced group as one
publication carrying every token, and still rejects a duplicated receipt.
Assisted-by: claude-fable-5.16 files changed, 61 insertions(+), 17 deletions(-)
crates/notebook/tests/sync_paragraph.rs+4-1| ... | @@ -70,7 +70,10 @@ fn locate_in<T>( | ... | @@ -70,7 +70,10 @@ fn locate_in<T>( |
| 70 | fn split_paragraph(page: &mut Page, text: ExGuid, offset: u32) { | 70 | fn split_paragraph(page: &mut Page, text: ExGuid, offset: u32) { |
| 71 | locate_in(page, text, |list, index| { | 71 | locate_in(page, text, |list, index| { |
| 72 | // One identity per split, numbered as OneNote numbers the paragraph it creates. | 72 | // One identity per split, numbered as OneNote numbers the paragraph it creates. |
| 73 | let guid = new_id().unwrap().guid; | 73 | // A GUID of the split's own: `new_id` shares its GUID across identities. |
| 74 | let ExGuid { mut guid, n } = new_id().unwrap(); | ||
| 75 | guid[14] ^= n as u8; | ||
| 76 | guid[15] ^= 0xff; | ||
| 74 | let mut right = list[index].clone(); | 77 | let mut right = list[index].clone(); |
| 75 | right.id = ExGuid { guid, n: 1 }; | 78 | right.id = ExGuid { guid, n: 1 }; |
| 76 | right.tags.clear(); | 79 | right.tags.clear(); |
crates/onestore/src/page/text.rs+16-7| ... | @@ -153,14 +153,23 @@ impl From<EditError> for crate::Error { | ... | @@ -153,14 +153,23 @@ impl From<EditError> for crate::Error { |
| 153 | } | 153 | } |
| 154 | } | 154 | } |
| 155 | 155 | ||
| 156 | /// A fresh random identity for a new paragraph or text object. | 156 | /// A fresh identity for a new paragraph or text object. Identities share one random GUID |
| 157 | /// A fresh identity with `n` 1: OneNote never stores an object as `{guid},0`, and OneNote | 157 | /// per 255 allocations, as OneNote's do within a session: an object's stored id table has |
| 158 | /// 2010 loses outline elements stored that way (corpus/math-edit/native-drop). | 158 | /// an entry per distinct GUID it references, in every revision that rewrites it. `n` starts |
| 159 | /// at 1: OneNote never stores an object as `{guid},0`, and OneNote 2010 loses outline | ||
| 160 | /// elements stored that way (corpus/math-edit/native-drop). | ||
| 159 | pub fn new_id() -> Result<ExGuid, EditError> { | 161 | pub fn new_id() -> Result<ExGuid, EditError> { |
| 160 | Ok(ExGuid { | 162 | static NEXT: std::sync::Mutex<Option<ExGuid>> = std::sync::Mutex::new(None); |
| 161 | guid: crate::write::fresh_guid().map_err(|_| EditError::Identity)?, | 163 | let mut next = NEXT.lock().map_err(|_| EditError::Identity)?; |
| 162 | n: 1, | 164 | let id = match *next { |
| 163 | }) | 165 | Some(id) => id, |
| 166 | None => ExGuid { | ||
| 167 | guid: crate::write::fresh_guid().map_err(|_| EditError::Identity)?, | ||
| 168 | n: 1, | ||
| 169 | }, | ||
| 170 | }; | ||
| 171 | *next = (id.n < 255).then_some(ExGuid { n: id.n + 1, ..id }); | ||
| 172 | Ok(id) | ||
| 164 | } | 173 | } |
| 165 | 174 | ||
| 166 | impl Paragraph { | 175 | impl Paragraph { |
fuzz/fuzz_targets/protected_write.rs+3-1| ... | @@ -45,7 +45,9 @@ fuzz_target!(|data: &[u8]| { | ... | @@ -45,7 +45,9 @@ fuzz_target!(|data: &[u8]| { |
| 45 | let stop = (start + u32::from(step[2]) % 8).min(end); | 45 | let stop = (start + u32::from(step[2]) % 8).min(end); |
| 46 | // Marked so its absence from the stored bytes is checkable. | 46 | // Marked so its absence from the stored bytes is checkable. |
| 47 | let inserted = format!("\u{1f512}sealed\u{1f512}{}", String::from_utf8_lossy(&step[3..]).replace('\0', "")); | 47 | let inserted = format!("\u{1f512}sealed\u{1f512}{}", String::from_utf8_lossy(&step[3..]).replace('\0', "")); |
| 48 | let format = text.format_at(start.min(end.saturating_sub(1))).unwrap().clone(); | 48 | let Ok(format) = text.format_at(start.min(end.saturating_sub(1))).cloned() else { |
| 49 | return; | ||
| 50 | }; | ||
| 49 | let edit = Edit { | 51 | let edit = Edit { |
| 50 | range: start..stop, | 52 | range: start..stop, |
| 51 | replacement: Paragraph::new(inserted.clone(), format), | 53 | replacement: Paragraph::new(inserted.clone(), format), |
tools/native_stress.py+1-1| ... | @@ -219,7 +219,7 @@ def exercise(output, shared, clients, action, wait_action, wait_text, checkpoint | ... | @@ -219,7 +219,7 @@ def exercise(output, shared, clients, action, wait_action, wait_text, checkpoint |
| 219 | (output / 'clocks.json').write_text(json.dumps(clocks, indent=2)) | 219 | (output / 'clocks.json').write_text(json.dumps(clocks, indent=2)) |
| 220 | rust = {actor: [json.loads(line) for line in (folder / f'{actor}.jsonl').read_text().splitlines()] for actor in processes} | 220 | rust = {actor: [json.loads(line) for line in (folder / f'{actor}.jsonl').read_text().splitlines()] for actor in processes} |
| 221 | commits, expected_rust = edit_history(rust, operations, edit, offline=config.get('offline', False)) | 221 | commits, expected_rust = edit_history(rust, operations, edit, offline=config.get('offline', False)) |
| 222 | assert len(commits) == rust_writers * operations, 'Missing Rust acknowledgements' | 222 | assert sum(event.get('operations', 1) for event in commits) == rust_writers * operations, 'Missing Rust acknowledgements' |
| 223 | native_events = [] | 223 | native_events = [] |
| 224 | for client in clients: | 224 | for client in clients: |
| 225 | local = client['folder'] / 'stress-events.jsonl' | 225 | local = client['folder'] / 'stress-events.jsonl' |
tools/offline_history.py+18-7| ... | @@ -20,7 +20,6 @@ def publication_links(logs, operations, partial=False): | ... | @@ -20,7 +20,6 @@ def publication_links(logs, operations, partial=False): |
| 20 | edits = [event for event in events if event['event'] == 'local_commit'] | 20 | edits = [event for event in events if event['event'] == 'local_commit'] |
| 21 | assert [event['operation'] for event in edits] == list(range(len(edits))), 'Missing or duplicate local acknowledgement' | 21 | assert [event['operation'] for event in edits] == list(range(len(edits))), 'Missing or duplicate local acknowledgement' |
| 22 | assert len(edits) <= operations and (partial or len(edits) == operations), 'Local operation count differs' | 22 | assert len(edits) <= operations and (partial or len(edits) == operations), 'Local operation count differs' |
| 23 | assert len({event['id'] for event in edits}) == len(edits), 'Duplicate local intent ID' | ||
| 24 | assert [event['id'] for event in edits] == sorted(event['id'] for event in edits), 'Local IDs went backwards' | 23 | assert [event['id'] for event in edits] == sorted(event['id'] for event in edits), 'Local IDs went backwards' |
| 25 | for event in edits: | 24 | for event in edits: |
| 26 | token = f' [{actor}:{event["operation"]}]' | 25 | token = f' [{actor}:{event["operation"]}]' |
| ... | @@ -37,16 +36,28 @@ def publication_links(logs, operations, partial=False): | ... | @@ -37,16 +36,28 @@ def publication_links(logs, operations, partial=False): |
| 37 | seen_revisions = set() | 36 | seen_revisions = set() |
| 38 | for actor, events in logs.items(): | 37 | for actor, events in logs.items(): |
| 39 | if not actor.startswith('w'): continue | 38 | if not actor.startswith('w'): continue |
| 40 | edits = {event['id']: event for event in events if event['event'] == 'local_commit'} | 39 | # A save replaces the newest pending save of its page, so consecutive operations |
| 41 | receipts = [event for event in events if event['event'] == 'remote_receipt'] | 40 | # can share one intent and one publication. |
| 42 | assert len({event['id'] for event in receipts}) == len(receipts), 'Duplicate remote receipt' | 41 | edits = {} |
| 42 | for event in events: | ||
| 43 | if event['event'] == 'local_commit': edits.setdefault(event['id'], []).append(event) | ||
| 44 | def once(rows): | ||
| 45 | # The client reports a coalesced intent's receipt once per operation. | ||
| 46 | found, seen = {}, {} | ||
| 47 | for row in rows: | ||
| 48 | assert found.setdefault(row['id'], row)['revision'] == row['revision'], 'One intent has two receipts' | ||
| 49 | seen[row['id']] = seen.get(row['id'], 0) + 1 | ||
| 50 | assert all(count == len(edits.get(id, [])) for id, count in seen.items()), 'Duplicate remote receipt' | ||
| 51 | return list(found.values()) | ||
| 52 | receipts = once(event for event in events if event['event'] == 'remote_receipt') | ||
| 43 | assert set(event['id'] for event in receipts) <= set(edits), 'Receipt lacks a local intent' | 53 | assert set(event['id'] for event in receipts) <= set(edits), 'Receipt lacks a local intent' |
| 44 | assert partial or len(receipts) == len(edits), 'Local success lacks remote acknowledgement' | 54 | assert partial or len(receipts) == len(edits), 'Local success lacks remote acknowledgement' |
| 45 | reopened = [event for event in events if event['event'] == 'reopened_receipt'] | 55 | reopened = once(event for event in events if event['event'] == 'reopened_receipt') |
| 46 | if not partial: | 56 | if not partial: |
| 47 | assert [(event['id'], event['revision']) for event in reopened] == [(event['id'], event['revision']) for event in receipts], 'Receipt changed across reopen' | 57 | assert [(event['id'], event['revision']) for event in reopened] == [(event['id'], event['revision']) for event in receipts], 'Receipt changed across reopen' |
| 48 | for receipt in receipts: | 58 | for receipt in receipts: |
| 49 | intent = edits[receipt['id']] | 59 | group = edits[receipt['id']] |
| 60 | intent = {**group[0], 'token': ''.join(event['token'] for event in group)} | ||
| 50 | attempts = [event for event in events if event['event'] == 'remote_attempt' and event['revision'] == receipt['revision']] | 61 | attempts = [event for event in events if event['event'] == 'remote_attempt' and event['revision'] == receipt['revision']] |
| 51 | assert len(attempts) == 1, 'Receipt does not identify one publication attempt' | 62 | assert len(attempts) == 1, 'Receipt does not identify one publication attempt' |
| 52 | attempt, = attempts | 63 | attempt, = attempts |
| ... | @@ -68,6 +79,6 @@ def publication_links(logs, operations, partial=False): | ... | @@ -68,6 +79,6 @@ def publication_links(logs, operations, partial=False): |
| 68 | assert attempt['after'] == attempt['before'] + intent['token'], 'Remote publication differs from local intent' | 79 | assert attempt['after'] == attempt['before'] + intent['token'], 'Remote publication differs from local intent' |
| 69 | tokens(attempt['after']) | 80 | tokens(attempt['after']) |
| 70 | assert attempt['before'] not in links, 'Remote publications branched from the same content' | 81 | assert attempt['before'] not in links, 'Remote publications branched from the same content' |
| 71 | event = {**attempt, 'event': 'commit', 'operation': intent['operation'], 'token': intent['token'], 'finished_us': receipt['at_us']} | 82 | event = {**attempt, 'event': 'commit', 'operation': group[-1]['operation'], 'operations': len(group), 'token': intent['token'], 'finished_us': receipt['at_us']} |
| 72 | links[attempt['before']] = event, attempt['after'] | 83 | links[attempt['before']] = event, attempt['after'] |
| 73 | return links | 84 | return links |
tools/test_offline_history.py+19| ... | @@ -25,6 +25,25 @@ class OfflineHistoryTests(unittest.TestCase): | ... | @@ -25,6 +25,25 @@ class OfflineHistoryTests(unittest.TestCase): |
| 25 | self.assertEqual([event['token'] for event in commits], [' [w1:0]', ' [w0:0]']) | 25 | self.assertEqual([event['token'] for event in commits], [' [w1:0]', ' [w0:0]']) |
| 26 | self.assertEqual(text, self.logs['r0'][1]['text']) | 26 | self.assertEqual(text, self.logs['r0'][1]['text']) |
| 27 | 27 | ||
| 28 | def test_a_save_that_replaced_a_pending_save_shares_its_intent_and_publication(self): | ||
| 29 | base = 'Concurrent edits:' | ||
| 30 | logs = {'w0': [ | ||
| 31 | {'event': 'ready', 'offline': True}, | ||
| 32 | {'event': 'local_commit', 'id': 1, 'operation': 0, 'before': base, 'token': ' [w0:0]', 'started_us': 1, 'finished_us': 2}, | ||
| 33 | {'event': 'local_commit', 'id': 1, 'operation': 1, 'before': base + ' [w0:0]', 'token': ' [w0:1]', 'started_us': 3, 'finished_us': 4}, | ||
| 34 | {'event': 'remote_attempt', 'revision': 'a', 'before': base, 'after': base + ' [w0:0] [w0:1]', 'state': 'Committed', 'started_us': 5, 'finished_us': 6}, | ||
| 35 | {'event': 'remote_receipt', 'id': 1, 'revision': 'a', 'at_us': 7}, | ||
| 36 | {'event': 'remote_receipt', 'id': 1, 'revision': 'a', 'at_us': 7}, | ||
| 37 | {'event': 'reopened_receipt', 'id': 1, 'revision': 'a'}, | ||
| 38 | {'event': 'reopened_receipt', 'id': 1, 'revision': 'a'}, | ||
| 39 | {'event': 'done'}, | ||
| 40 | ], 'r0': [{'event': 'ready'}, {'event': 'done'}]} | ||
| 41 | commits, text = edit_history(logs, 2, offline=True) | ||
| 42 | self.assertEqual([(event['token'], event['operations']) for event in commits], [(' [w0:0] [w0:1]', 2)]) | ||
| 43 | self.assertEqual(text, base + ' [w0:0] [w0:1]') | ||
| 44 | logs['w0'][3]['after'] = base + ' [w0:1]' | ||
| 45 | with self.assertRaises(AssertionError): edit_history(logs, 2, offline=True) | ||
| 46 | |||
| 28 | def test_false_receipts_lost_local_intents_and_changed_reopen_state_fail(self): | 47 | def test_false_receipts_lost_local_intents_and_changed_reopen_state_fail(self): |
| 29 | for index, field, value in [(1, 'id', 2), (1, 'token', ' [w0:9]'), (3, 'state', 'Unknown'), | 48 | for index, field, value in [(1, 'id', 2), (1, 'token', ' [w0:9]'), (3, 'state', 'Unknown'), |
| 30 | (3, 'after', 'Concurrent edits: [w1:0]'), (4, 'at_us', 0), | 49 | (3, 'after', 'Concurrent edits: [w1:0]'), (4, 'at_us', 0), |