Spill file_id seed fails open: a failed cold-dir scan restarts numbering at 1 and the next spill renames over a live heap file

[TinDang97]

Summary

eviction::next_spill_file_id_seed fails open: when it cannot scan the cold data
directory it returns 1 and logs a warning, even though higher-numbered spill files
exist on disk. The spill counter then restarts low, and because a spill writes to
{shard_dir}/data/heap-{file_id:06}.mpf via a .tmp + rename, the next spill
silently replaces an existing file of the same name — destroying cold data that is still
referenced.

This is the same defect class as moon#893 (Warm vector segment file_id reuse makes boot recovery delete the segment directory it just attached), in the KV cold tier rather than
the vector engine. That one is tracked; this one was not.

The two paths that reset the seed

src/storage/eviction.rs:954-989:

let entries = match std::fs::read_dir(&data_dir) {
    Ok(e) => e,
    Err(e) => {
        if e.kind() != std::io::ErrorKind::NotFound {
            tracing::warn!(..., "spill file_id seed: could not scan cold dir; defaulting to 1");
        }
        return 1;          // <-- higher-numbered files may exist RIGHT NOW
    }
};

1. read_dir fails with anything other than NotFound — permissions, transient I/O, a
   full or wedged filesystem. Warns, returns 1.
2. Any heap-*.mpf whose name does not parse is skipped, so max_id can come back below
   the true maximum even on a successful scan.

NotFound really is the benign fresh-server case and is correctly silent. The other
errors are not benign, and 1 is the most dangerous possible answer to them.

Why the consequence is data loss, not just confusion

src/storage/tiered/kv_spill.rs:441-450 writes the batch to heap-{file_id:06}.tmp,
fsyncs, then renames onto heap-{file_id:06}.mpf. A POSIX rename over an existing path
replaces it silently. So with a reset seed:

disk before:  heap-000001.mpf (live, referenced by the cold index)
              ... heap-000050.mpf
seed resets to 1
next spill:   writes heap-000001.tmp, renames onto heap-000001.mpf
disk after:   the original heap-000001.mpf is GONE

Every key whose only copy lived in the replaced file is unrecoverable, and the cold index
still points at it. Reads for those keys fail or return wrong data after the next restart.

How this surfaced

Found while reviewing PR #996 (moon#983), which makes cold-index duplicate resolution
order by file_id instead of manifest push order. That fix's correctness argument rests on
file_id being monotonic across restarts, so I checked the seeding function.

To be clear about scope: this does not weaken PR #996 and should not block it. Under a
reset seed the older copy is destroyed by the rename before ordering is ever consulted,
so there are not two copies to order — the ordering fix makes no scenario worse. The
author reached the same conclusion independently and documented the caveat on
ColdLocation::recency_key rather than expanding scope, which was the right call. This is
the separate follow-up.

Suggested fix

Do not fail open on a durability-critical counter. On a non-NotFound scan error, either
propagate the error or refuse to start, rather than returning a seed that is known-unsafe.
Note the author's finding that there is no error path out of shard init today, so this may
need one — which is itself worth knowing.

Two cheaper hardening steps that stand on their own:

• Refuse to clobber. Create the final file with O_EXCL semantics, or check for
  existence before the rename, so a seed bug degrades into a loud error instead of silent
  destruction. A spill overwriting an existing heap-*.mpf is never correct, regardless of
  how the file_id was chosen.
• Count unparseable heap-* names during the scan and refuse to seed (rather than
  under-seed) if any are present.

The first is the valuable one: it converts this entire class — including any future
file_id bug — from data loss into a startup failure.