Skip to content

open() replaces a corrupt store with an empty one instead of failing #9

Description

@mkhairi

Summary

A single unreadable page on open makes PagedbStorage::open rename the whole store aside and create an empty one in its place, then report healthy. There is no opt-out, no callback, and no error the embedder can catch and refuse. This cost a live 15 GB deployment its entire dataset.

nodedb-lite/src/storage/pagedb_storage.rs:246,277 (present on 6b575ae).

What happened

Reopening a 15 GB store produced three log lines and an empty database:

ERROR pagedb::pager::core: page AEAD/MAC verification failed on read page_id=89099 file=Main binding=Node
ERROR nodedb_lite::storage::pagedb_storage: pagedb open detected corruption — recovering (rename corrupt
      store aside, recreate fresh). A full re-sync from Origin is required to recover data.
ERROR nodedb_lite::storage::pagedb_storage: renaming corrupted pagedb store to backup and creating a fresh
      database path=…/aql.db corrupt_backup=…/aql.corrupt.1785382598

30 ms elapsed between the failed read and the fresh database. health() then reported healthy and the service began accepting writes into the empty store, so a supervisor restart turned "one bad page" into "no data, and now diverging from the copy that was set aside".

The bad page was confirmed with pagedb-fsck to have been already unreadable in a copy taken before the restart — so the store had been in this state for some unknown period while the process ran on already-loaded state, and the reopen was simply the first read of that page.

Why the recovery does not fit a local-first embedder

The message says a full re-sync from Origin is required. An embedder that never calls start_sync has no Origin, so "recover" means "discard". The choice is made unilaterally at open, before the caller sees anything, and the only reason the data still exists at all is that the rename happens to preserve it under a different name — which is upstream's implementation detail, not a contract a caller can rely on.

Suggested change

  1. Fail closed by default — return the corruption error from open and let the embedder decide. Most callers would rather refuse to start than start empty.
  2. If the auto-recovery is worth keeping, gate it behind an explicit opt-in (e.g. recover_by_discarding_store: bool on the open options, default false), and skip it entirely when no sync origin is configured — that is exactly the case where discarding is unrecoverable.

For what it's worth, pagedb has recently moved the other way for a comparable fault: 501f2be feat(recovery): detach an unreadable free list instead of failing open degrades and keeps the data. The two behaviours are inconsistent for faults of similar severity.

Reproducing

Corrupt one page of a scratch store, open it through PagedbStorage, and observe that open succeeds with an empty database rather than returning an error. A fix should make that open fail.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions