From 61e5924c3e82e25ec18a37c7d76a0dced9e9c95a Mon Sep 17 00:00:00 2001
From: Franz Heinzmann <frando@unbiskant.org>
Date: Wed, 8 Nov 2023 00:14:47 +0100
Subject: [PATCH] fix(iroh-sync): prevent panic in namespace migration (#1775)

## Description

The migration as commited with #1770 panics when starting iroh with an
existing data dir. The reason is that redb panics when deleting table in
a transaction where they were opened as well. See:
https://github.com/cberner/redb/issues/715

This fixes this by moving the table deletion to a separate transaction.

The limitation and potential panic in redb was fixed in
https://github.com/cberner/redb/pull/716 so once we upgrade to the next
(still unreleased) version of redb, this can be removed again.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
---
 iroh-sync/src/store/fs/migrations.rs | 33 ++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/iroh-sync/src/store/fs/migrations.rs b/iroh-sync/src/store/fs/migrations.rs
index 7e6901a7..8c1e9c43 100644
--- a/iroh-sync/src/store/fs/migrations.rs
+++ b/iroh-sync/src/store/fs/migrations.rs
@@ -14,8 +14,9 @@ use super::{
 /// Run all database migrations, if needed.
 pub fn run_migrations(db: &Database) -> Result<()> {
     run_migration(db, migration_001_populate_latest_table)?;
-    run_migration(db, migration_002_namespaces_v2)?;
-    run_migration(db, migration_003_populate_by_key_index)?;
+    run_migration(db, migration_002_namespaces_populate_v2)?;
+    run_migration(db, migration_003_namespaces_delete_v1)?;
+    run_migration(db, migration_004_populate_by_key_index)?;
     Ok(())
 }
 
@@ -74,12 +75,11 @@ fn migration_001_populate_latest_table(tx: &WriteTransaction) -> Result<MigrateO
     Ok(MigrateOutcome::Execute(len))
 }
 
-/// Migrate the namespaces table from V1 to V2.
-fn migration_002_namespaces_v2(tx: &WriteTransaction) -> Result<MigrateOutcome> {
+/// Copy the namespaces data from V1 to V2.
+fn migration_002_namespaces_populate_v2(tx: &WriteTransaction) -> Result<MigrateOutcome> {
     let namespaces_v1_exists = tx
         .list_tables()?
         .any(|handle| handle.name() == NAMESPACES_TABLE_V1.name());
-    // migration 002: update namespaces from V1 to V2
     if !namespaces_v1_exists {
         return Ok(MigrateOutcome::Skip);
     }
@@ -95,12 +95,31 @@ fn migration_002_namespaces_v2(tx: &WriteTransaction) -> Result<MigrateOutcome>
         namespaces_v2.insert(&id, (raw_kind, &raw_bytes))?;
         entries += 1;
     }
-    tx.delete_table(NAMESPACES_TABLE_V1)?;
     Ok(MigrateOutcome::Execute(entries))
 }
 
+/// Delete the v1 namespaces table.
+///
+/// This should be part of [`migration_002_namespaces_populate_v2`] but due to a limitation in
+/// [`redb`] up to v1.3.0 a table cannot be deleted in a transaction that also opens this table.
+/// Therefore the table deletion has to be in a separate transaction.
+///
+/// This limitation was removed in <https://github.com/cberner/redb/pull/716> so this can be merged
+/// back into [`migration_002_namespaces_populate_v2`] once we upgrade to the next redb version
+/// after 1.3.
+fn migration_003_namespaces_delete_v1(tx: &WriteTransaction) -> Result<MigrateOutcome> {
+    let namespaces_v1_exists = tx
+        .list_tables()?
+        .any(|handle| handle.name() == NAMESPACES_TABLE_V1.name());
+    if !namespaces_v1_exists {
+        return Ok(MigrateOutcome::Skip);
+    }
+    tx.delete_table(NAMESPACES_TABLE_V1)?;
+    Ok(MigrateOutcome::Execute(1))
+}
+
 /// migration 003: populate the by_key index table(which did not exist before)
-fn migration_003_populate_by_key_index(tx: &WriteTransaction) -> Result<MigrateOutcome> {
+fn migration_004_populate_by_key_index(tx: &WriteTransaction) -> Result<MigrateOutcome> {
     let mut by_key_table = tx.open_table(RECORDS_BY_KEY_TABLE)?;
     let records_table = tx.open_table(RECORDS_TABLE)?;
     if !by_key_table.is_empty()? || records_table.is_empty()? {