Skip to content

dolt_gc() relies on rename causing potential durability issues on Windows #1048

Description

@timsehn

Problem

dolt_gc() currently compacts the chunk store by writing a new temporary file and replacing the live database file with rename(). That gives us a simple atomic-replace path on POSIX, but it is not the same safety model SQLite uses for VACUUM.

SQLite avoids delete/rename for normal VACUUM: it copies compacted pages back into the existing database file through the pager/btree layer under a write transaction. The pager journals overwritten pages, so crash recovery can roll back or complete coherently. It also preserves the database file identity for existing connections.

Doltlite GC is different because compaction moves chunks into the compacted/oldgen region, rewrites offsets, rewrites the chunk index, and updates the manifest. Existing connections may hold an open file handle and an in-memory chunk index with old offsets.

The rename model creates sharp edges:

  • Existing POSIX readers can keep reading the old unlinked inode while new opens see the compacted file.
  • If replacement succeeds but a later reopen/sync step fails, the connection can become hard to reason about.
  • Raw rename() is not exposed through SQLite VFS, so the final replacement bypasses custom VFS semantics.
  • Windows/custom VFS behavior is not equivalent to POSIX atomic replacement.
  • The approach does not match SQLite's pager-backed durability model.

PR #1047 hardens one failure window by closing the old handle once replacement succeeds and adopting the compacted in-memory metadata if replacement already happened. That reduces split-brain risk, but it does not change the underlying rename-based design.

Proposed direction

Consider a format bump that removes rename from the normal GC commit path and moves GC to an append-only generation model:

  1. Write the new compacted generation elsewhere in the same file.
  2. Write and validate the new chunk index for that generation.
  3. Validate that every chunk referenced by the new root/refs is present before accepting the new generation.
  4. Atomically flip a small manifest/superblock pointer to make the new generation current.
  5. Leave the old generation reachable for existing readers until a later cleanup pass can reclaim it safely.

This is closer to content-addressed storage than SQLite's page-overwrite model. It avoids replacing the database file, avoids stale file-handle split-brain, and can be implemented using SQLite VFS open/write/sync primitives except for any platform-specific file truncation/cleanup details.

Open questions

  • Do we need two manifest/superblock slots with checksums and monotonically increasing generation numbers?
  • How should old generations be reclaimed when another process may still have an old file/index snapshot?
  • What locks are required to make the generation flip safe across processes?
  • Should crash recovery validate all chunks referenced by the selected root before accepting that root?
  • Can this share machinery with WAL replay/root validation?

Acceptance criteria

  • dolt_gc() does not replace the live database file with rename() on the normal path.
  • Crash during GC leaves either the old generation or the new generation selected, never a partially selected generation.
  • Existing open readers do not read compacted bytes using stale offsets.
  • Fresh opens after crash validate the selected generation before use.
  • The implementation uses SQLite VFS APIs for file access wherever SQLite exposes the needed operation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions