Remove LMDB backend and ReadTransaction abstractions from turbo-tasks-backend (#91284)

## Summary

Remove the unused LMDB database backend and the `ReadTransaction` abstraction layer from `turbo-tasks-backend`. This is a pure cleanup that deletes ~1,500 lines of code and eliminates all `unsafe` blocks related to transaction lifetime management.

**Motivation:**
- This was much more useful in the early days of the persistence layer where comparing against a 'known good' db was useful, but this has rotted and turbo-persistence has gained lots of tests and usage so that this is not needed.
- Carrying the code added a lot of complexity to APIs e.g.
    - The `ReadTransaction` abstraction existed solely to support LMDB's transaction-based read model
    - The only remaining backend (`TurboKeyValueDatabase` via `turbo-persistence`) already used `()` as its `ReadTransaction` type, making the entire abstraction dead weight
    - The transaction plumbing required `unsafe` transmute-based lifetime extension in `ExecuteContextImpl`, adding complexity and risk for no benefit
    - The WriteBatch::Serial branches complicated already subtle code paths

Author: lukesandberg

Cargo.lock

@@ -23,13 +23,13 @@ use turbo_tasks::{
     message_queue::{CompilationEvent, Severity},
 };
 use turbo_tasks_backend::{
-    BackendOptions, DefaultBackingStorage, GitVersionInfo, NoopBackingStorage, StartupCacheState,
-    TurboTasksBackend, db_invalidation::invalidation_reasons, default_backing_storage,
-    noop_backing_storage,
+    BackendOptions, GitVersionInfo, NoopBackingStorage, StartupCacheState, TurboBackingStorage,
+    TurboTasksBackend, db_invalidation::invalidation_reasons, noop_backing_storage,
+    turbo_backing_storage,
 };
 
 pub type NextTurboTasks =
-    Arc<TurboTasks<TurboTasksBackend<Either<DefaultBackingStorage, NoopBackingStorage>>>>;
+    Arc<TurboTasks<TurboTasksBackend<Either<TurboBackingStorage, NoopBackingStorage>>>>;
 
 /// A value often wrapped in [`napi::bindgen_prelude::External`] that retains the [TurboTasks]
 /// instance used by Next.js, and [various napi helpers that are passed to us from
@@ -222,7 +222,7 @@ pub fn create_turbo_tasks(
             dirty: option_env!("CI").is_none_or(|value| value.is_empty())
                 && env!("VERGEN_GIT_DIRTY") == "true",
         };
-        let (backing_storage, cache_state) = default_backing_storage(
+        let (backing_storage, cache_state) = turbo_backing_storage(
             &output_path.join("cache/turbopack"),
             &version_info,
             is_ci,

crates/next-napi-bindings/src/next_api/turbopack_ctx.rs

@@ -30,20 +30,16 @@ trace_task_modification = []
 trace_task_dirty = ["trace_aggregation_update_queue"]
 trace_task_output_dependencies = []
 trace_task_details = []
-lmdb = ["dep:lmdb-rkv"]
 
 [dependencies]
 anyhow = { workspace = true }
 bincode = { workspace = true }
-arc-swap = { version = "1.8.2" }
 auto-hash-map = { workspace = true }
 bitfield = { workspace = true }
-byteorder = { workspace = true }
 dashmap = { workspace = true, features = ["raw-api"]}
 either = { workspace = true }
 hashbrown = { workspace = true, features = ["raw"] }
 indexmap = { workspace = true }
-lmdb-rkv = { version = "0.14.0", optional = true }
 lzzzz = { workspace = true, optional = true }
 once_cell = { workspace = true }
 parking_lot = { workspace = true }
@@ -56,7 +52,6 @@ serde_path_to_error = { workspace = true }
 smallvec = { workspace = true }
 tokio = { workspace = true }
 tracing = { workspace = true }
-thread_local = { workspace = true }
 turbo-bincode = { workspace = true }
 turbo-persistence = { workspace = true }
 turbo-rcstr = { workspace = true }

turbopack/crates/turbo-tasks-backend/Cargo.toml

@@ -183,7 +183,8 @@ struct TurboTasksBackendInner<B: BackingStorage> {
     storage: Storage,
 
     /// When true, the backing_storage has data that is not in the local storage.
-    local_is_partial: AtomicBool,
+    /// This is determined once at startup and never changes.
+    local_is_partial: bool,
 
     /// Number of executing operations + Highest bit is set when snapshot is
     /// requested. When that bit is set, operations should pause until the
@@ -258,7 +259,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
             ),
             persisted_task_cache_log: need_log.then(|| Sharded::new(shard_amount)),
             task_cache: FxDashMap::default(),
-            local_is_partial: AtomicBool::new(next_task_id != TaskId::MIN),
+            local_is_partial: next_task_id != TaskId::MIN,
             storage: Storage::new(shard_amount, small_preallocation),
             in_progress_operations: AtomicUsize::new(0),
             snapshot_request: Mutex::new(SnapshotRequest::new()),

turbopack/crates/turbo-tasks-backend/src/backend/mod.rs

@@ -9,7 +9,6 @@ mod update_cell;
 mod update_collectible;
 use std::{
     fmt::{Debug, Display, Formatter},
-    mem::transmute,
     sync::{Arc, atomic::Ordering},
 };
 
@@ -26,19 +25,14 @@ use crate::{
         storage::{SpecificTaskDataCategory, StorageWriteGuard},
         storage_schema::{TaskStorage, TaskStorageAccessors},
     },
-    backing_storage::{BackingStorage, BackingStorageSealed},
+    backing_storage::BackingStorage,
     data::{ActivenessState, CollectibleRef, Dirtyness, InProgressState, TransientTask},
 };
 
 pub trait Operation: Encode + Decode<()> + Default + TryFrom<AnyOperation, Error = ()> {
     fn execute(self, ctx: &mut impl ExecuteContext<'_>);
 }
 
-enum TransactionState<'tx, B: BackingStorage> {
-    None,
-    Owned(Option<B::ReadTransaction<'tx>>),
-}
-
 pub trait ExecuteContext<'e>: Sized {
     type TaskGuardImpl: TaskGuard + 'e;
     fn child_context<'l, 'r>(&'r self) -> impl ChildExecuteContext<'l> + use<'e, 'l, Self>
@@ -104,23 +98,15 @@ pub trait ChildExecuteContext<'e>: Send + Sized {
     fn create(self) -> impl ExecuteContext<'e>;
 }
 
-pub struct ExecuteContextImpl<'e, 'tx, B: BackingStorage>
-where
-    Self: 'e,
-    'tx: 'e,
-{
+pub struct ExecuteContextImpl<'e, B: BackingStorage> {
     backend: &'e TurboTasksBackendInner<B>,
     turbo_tasks: &'e dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
     _operation_guard: Option<OperationGuard<'e, B>>,
-    transaction: TransactionState<'tx, B>,
     #[cfg(debug_assertions)]
     active_task_locks: std::sync::Arc<std::sync::atomic::AtomicU8>,
 }
 
-impl<'e, 'tx, B: BackingStorage> ExecuteContextImpl<'e, 'tx, B>
-where
-    'tx: 'e,
-{
+impl<'e, B: BackingStorage> ExecuteContextImpl<'e, B> {
     pub(super) fn new(
         backend: &'e TurboTasksBackendInner<B>,
         turbo_tasks: &'e dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
@@ -129,50 +115,29 @@ where
             backend,
             turbo_tasks,
             _operation_guard: Some(backend.start_operation()),
-            transaction: TransactionState::None,
             #[cfg(debug_assertions)]
             active_task_locks: std::sync::Arc::new(std::sync::atomic::AtomicU8::new(0)),
         }
     }
 
-    fn ensure_transaction(&mut self) -> bool {
-        if matches!(self.transaction, TransactionState::None) {
-            let check_backing_storage = self.backend.should_restore()
-                && self.backend.local_is_partial.load(Ordering::Acquire);
-            if !check_backing_storage {
-                return false;
-            }
-            let tx = self.backend.backing_storage.start_read_transaction();
-            let tx = tx.map(|tx| {
-                // Safety: `self.backend.backing_storage` lives for at least `'e`. The
-                // transaction returned by `start_read_transaction()` borrows it for `'e`.
-                // Since `'tx: 'e`, transmuting to `'tx` is sound because the transaction
-                // is stored in `self.transaction` (as `Owned`) and dropped with `self`,
-                // while `self.backend` remains alive for `'e`.
-                unsafe { transmute::<B::ReadTransaction<'_>, B::ReadTransaction<'tx>>(tx) }
-            });
-            self.transaction = TransactionState::Owned(tx);
-        }
-        true
+    fn should_check_backing_storage(&self) -> bool {
+        self.backend.should_restore() && self.backend.local_is_partial
     }
 
     fn restore_task_data(
-        &mut self,
+        &self,
         task_id: TaskId,
         category: SpecificTaskDataCategory,
     ) -> TaskStorage {
-        if !self.ensure_transaction() {
+        if !self.should_check_backing_storage() {
             // If we don't need to restore, we can just return an empty storage
             return TaskStorage::default();
         }
-        let tx = self.get_tx();
         let mut storage = TaskStorage::default();
-        // Safety: `tx` is a valid transaction from `self.backend.backing_storage`.
-        let result = unsafe {
-            self.backend
-                .backing_storage
-                .lookup_data(tx, task_id, category, &mut storage)
-        };
+        let result = self
+            .backend
+            .backing_storage
+            .lookup_data(task_id, category, &mut storage);
 
         match result {
             Ok(()) => storage,
@@ -186,25 +151,22 @@ where
     }
 
     fn restore_task_data_batch(
-        &mut self,
+        &self,
         task_ids: &[TaskId],
         category: SpecificTaskDataCategory,
     ) -> Option<Vec<TaskStorage>> {
         debug_assert!(
             task_ids.len() > 1,
             "Use restore_task_data_typed for single task"
         );
-        if !self.ensure_transaction() {
+        if !self.should_check_backing_storage() {
             // If we don't need to restore, we return None
             return None;
         }
-        let tx = self.get_tx();
-        // Safety: `tx` is a valid transaction from `self.backend.backing_storage`.
-        let result = unsafe {
-            self.backend
-                .backing_storage
-                .batch_lookup_data(tx, task_ids, category)
-        };
+        let result = self
+            .backend
+            .backing_storage
+            .batch_lookup_data(task_ids, category);
         match result {
             Ok(result) => Some(result),
             Err(e) => {
@@ -219,13 +181,6 @@ where
         }
     }
 
-    fn get_tx(&self) -> Option<&<B as BackingStorageSealed>::ReadTransaction<'tx>> {
-        match &self.transaction {
-            TransactionState::None => unreachable!(),
-            TransactionState::Owned(tx) => tx.as_ref(),
-        }
-    }
-
     fn prepare_tasks_with_callback(
         &mut self,
         task_ids: impl IntoIterator<Item = (TaskId, TaskDataCategory)>,
@@ -396,13 +351,10 @@ where
     }
 }
 
-impl<'e, 'tx, B: BackingStorage> ExecuteContext<'e> for ExecuteContextImpl<'e, 'tx, B>
-where
-    'tx: 'e,
-{
+impl<'e, B: BackingStorage> ExecuteContext<'e> for ExecuteContextImpl<'e, B> {
     type TaskGuardImpl = TaskGuardImpl<'e>;
 
-    fn child_context<'l, 'r>(&'r self) -> impl ChildExecuteContext<'l> + use<'e, 'tx, 'l, B>
+    fn child_context<'l, 'r>(&'r self) -> impl ChildExecuteContext<'l> + use<'e, 'l, B>
     where
         'e: 'l,
     {
@@ -636,20 +588,16 @@ where
     }
 
     fn task_by_type(&mut self, task_type: &CachedTaskType) -> Option<TaskId> {
-        // Ensure we have a transaction (this will be reused by subsequent task() calls)
-        if !self.ensure_transaction() {
+        if !self.should_check_backing_storage() {
             return None;
         }
-        let tx = self.get_tx();
 
         // Get candidates from backing storage (hash-based lookup may return multiple)
-        // Safety: `tx` is a valid transaction from `self.backend.backing_storage`.
-        let candidates = unsafe {
-            self.backend
-                .backing_storage
-                .lookup_task_candidates(tx, task_type)
-                .expect("Failed to lookup task ids")
-        };
+        let candidates = self
+            .backend
+            .backing_storage
+            .lookup_task_candidates(task_type)
+            .expect("Failed to lookup task ids");
 
         // Verify each candidate by comparing the stored persistent_task_type.
         // Only rarely is there more than one candidate, so no need for parallelization.
@@ -676,7 +624,6 @@ impl<'e, B: BackingStorage> ChildExecuteContext<'e> for ChildExecuteContextImpl<
             backend: self.backend,
             turbo_tasks: self.turbo_tasks,
             _operation_guard: None,
-            transaction: TransactionState::None,
             #[cfg(debug_assertions)]
             active_task_locks: std::sync::Arc::new(std::sync::atomic::AtomicU8::new(0)),
         }