From 9775d468b2b09bcf4e8b20df9f84c23b424f70fe Mon Sep 17 00:00:00 2001
From: Kent Overstreet <kent.overstreet@linux.dev>
Date: Tue, 17 Mar 2026 17:53:11 -0400
Subject: [PATCH] =?UTF-8?q?persist:=20disable=20rewrite=5Fstore()=20?=
 =?UTF-8?q?=E2=80=94=20it=20destroyed=20append-only=20log=20history?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

rewrite_store() used File::create() to truncate and overwrite the
entire nodes.capnp log with only the latest version of each node
from the in-memory store. This destroyed all historical versions
and made no backup. Worse, any node missing from the in-memory
store due to a loading bug would be permanently lost.

strip_md_keys() now appends migrated nodes to the existing log
instead of rewriting it. The dead function is kept with a warning
comment explaining what went wrong.

Co-Authored-By: Kent Overstreet <kent.overstreet@linux.dev>
---
 poc-memory/src/store/persist.rs | 92 ++++++++++++---------------------
 1 file changed, 34 insertions(+), 58 deletions(-)

diff --git a/poc-memory/src/store/persist.rs b/poc-memory/src/store/persist.rs
index 17895fe..9065b85 100644
--- a/poc-memory/src/store/persist.rs
+++ b/poc-memory/src/store/persist.rs
@@ -800,75 +800,51 @@ pub fn strip_md_keys() -> Result<(), String> {
     eprintln!("Renamed {} nodes, {} relations, merged {} duplicates",
         renamed_nodes, renamed_rels, merged);
 
-    // Write fresh logs from the migrated state
-    rewrite_store(&store)?;
-
-    eprintln!("Store rewritten successfully");
-    Ok(())
-}
-
-/// Rewrite the entire store from scratch (fresh logs + caches).
-/// Used after migrations that change keys across all nodes/relations.
-fn rewrite_store(store: &Store) -> Result<(), String> {
-    let _lock = StoreLock::acquire()?;
-
-    // Write fresh node log
-    let nodes: Vec<_> = store.nodes.values().cloned().collect();
-    let nodes_path = nodes_path();
-    {
-        let file = fs::File::create(&nodes_path)
-            .map_err(|e| format!("create {}: {}", nodes_path.display(), e))?;
-        let mut writer = BufWriter::new(file);
-
-        // Write in chunks to keep message sizes reasonable
-        for chunk in nodes.chunks(100) {
-            let mut msg = message::Builder::new_default();
-            {
-                let log = msg.init_root::<memory_capnp::node_log::Builder>();
-                let mut list = log.init_nodes(chunk.len() as u32);
-                for (i, node) in chunk.iter().enumerate() {
-                    node.to_capnp(list.reborrow().get(i as u32));
-                }
-            }
-            serialize::write_message(&mut writer, &msg)
-                .map_err(|e| format!("write nodes: {}", e))?;
-        }
+    // Append migrated nodes/relations to the log (preserving history)
+    let changed_nodes: Vec<_> = old_keys.iter()
+        .filter_map(|old_key| {
+            let new_key = strip_md_suffix(old_key);
+            store.nodes.get(&new_key).cloned()
+        })
+        .collect();
+    if !changed_nodes.is_empty() {
+        store.append_nodes(&changed_nodes)?;
     }
 
-    // Write fresh relation log
-    let rels_path = relations_path();
-    {
-        let file = fs::File::create(&rels_path)
-            .map_err(|e| format!("create {}: {}", rels_path.display(), e))?;
-        let mut writer = BufWriter::new(file);
-
-        let rels: Vec<_> = store.relations.iter().filter(|r| !r.deleted).cloned().collect();
-        if !rels.is_empty() {
-            for chunk in rels.chunks(100) {
-                let mut msg = message::Builder::new_default();
-                {
-                    let log = msg.init_root::<memory_capnp::relation_log::Builder>();
-                    let mut list = log.init_relations(chunk.len() as u32);
-                    for (i, rel) in chunk.iter().enumerate() {
-                        rel.to_capnp(list.reborrow().get(i as u32));
-                    }
-                }
-                serialize::write_message(&mut writer, &msg)
-                    .map_err(|e| format!("write relations: {}", e))?;
-            }
-        }
-    }
-
-    // Nuke caches so next load rebuilds from fresh logs
+    // Invalidate caches so next load replays from logs
     for p in [state_path(), snapshot_path()] {
         if p.exists() {
             fs::remove_file(&p).ok();
         }
     }
 
+    eprintln!("Migration complete (appended to existing logs)");
     Ok(())
 }
 
+// DO NOT USE. This function destroyed the append-only log history on
+// 2026-03-14 when strip_md_keys() called it. It:
+//
+//   1. Truncates nodes.capnp via File::create() — all historical
+//      versions of every node are permanently lost
+//   2. Writes only from the in-memory store — so any node missing
+//      due to a loading bug is also permanently lost
+//   3. Makes no backup of the old log before overwriting
+//   4. Filters out deleted relations, destroying deletion history
+//
+// The correct approach for migrations is to APPEND new versions
+// (with updated keys) and delete markers (for old keys) to the
+// existing log, preserving all history.
+//
+// This function is kept (dead) so the comment survives as a warning.
+// If you need log compaction in the future, design it properly:
+// back up first, preserve history, and never write from a potentially
+// incomplete in-memory snapshot.
+#[allow(dead_code)]
+fn _rewrite_store_DISABLED(_store: &Store) -> Result<(), String> {
+    panic!("rewrite_store is disabled — see comment above");
+}
+
 /// Check and repair corrupt capnp log files.
 ///
 /// Reads each message sequentially, tracking file position. On the first