Skip to content
This repository was archived by the owner on Aug 3, 2026. It is now read-only.
This repository was archived by the owner on Aug 3, 2026. It is now read-only.

Persistence engine has no graceful shutdown: abort-on-drop can still tear an in-flight batch #162

Description

@pathscale

Context

The wait-for-ops race fix (patch 0014 of the pending fix series, src/persistence/task.rs) closed two problems:

  1. PersistenceTask::wait_for_ops could return while a popped operation was still in flight — between Queue::pop's len.fetch_sub and the engine loop's in_progress store-back, all three wait triggers held at once. The pop now sets the busy flag under the queue mutex before decrementing len, so the empty+idle state is never observable with an op in flight. (Observed as rare flakes: test_insert_primary_duplicate 1/30 under full-suite load reading data = 1000 instead of 100 from a reused pk after a stale pk_gen_state reload; test_space_insert_many_sync with pk_gen state 4917 vs 5000.)
  2. The engine task's AbortHandle was stored but never used, so a dropped table's engine task kept running — and writing — indefinitely. PersistenceTask now aborts it in Drop.

Residual hazard

Abort-on-drop narrows the problem but cannot fully eliminate it: tokio::task::AbortHandle::abort cancels at the task's next await point. If that point is inside PersistenceEngine::apply_batch_operation, the batch is torn — part of it is on disk, part isn't.

This is the ceiling for what a Drop impl can do:

  • Drop::drop is synchronous. It cannot .await the engine's JoinHandle, and block_on inside it panics/deadlocks when the drop happens on the runtime driving the engine task (which is where tables get dropped).
  • The two behaviors reachable from Drop are both imperfect: abort stops promptly but may tear a batch; a graceful shutdown flag checked at batch boundaries never tears a batch but lets the old engine keep writing for up to one batch after the table is gone — re-opening a bounded version of the stale-writer overlap the fix exists to close.

Abort was chosen because every reopen path that matters calls wait_for_ops first; with the fixed triggers the engine is then parked at the idle queue pop, where cancellation is clean. The torn-write case only arises for callers that drop without waiting — and those were already discarding queued operations.

Note the torn-batch state is identical to the process being killed mid-write: power loss and kill -9 produce it too, and no Drop impl helps with those.

Proposed work

  1. Explicit graceful closeasync fn close(self) on PersistenceTask, plumbed through codegen to the table API: signal shutdown, let an in-flight batch finish, await the engine task's exit. Drop's abort stays as the backstop for callers that never call it. Gives well-behaved callers zero overlap and zero torn writes. The vacuum-sink path is the realistic beneficiary. This is patch-0015-shaped: an API addition, separate from the race fix.
  2. Crash-atomic batch application (separate roadmap item) — a write-ahead journal or shadow-page scheme in the space format so a half-applied batch is detected and discarded on load. This is the only complete fix, because it also covers real crashes, not just drops. It changes the on-disk format, so it needs its own compatibility story and should not ride in a bug-fix series.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions