# DiskLoc.file_id u16 → u32 widening

**Status:** draft
**Date:** 2026-05-17
**Scope:** `armdb/src/disk_loc.rs`, `armdb/src/shard.rs`, `armdb/src/compaction.rs`,
`armdb/src/var_tree.rs`, `armdb/src/var_map.rs`, `armdb/src/const_tree.rs`,
`armdb/src/const_map.rs`, `armdb/src/recovery.rs`
**Source review:** `armdb/docs/reviews/codex-vartree-varmap.md` — issue #8
(`DiskLoc.file_id` truncates internal `u32` file IDs to `u16`)

## Context

`DiskLoc.file_id` is `u16` (`armdb/src/disk_loc.rs:7`), while every other
internal file ID is already `u32`:

- `Shard::next_file_id: u32` (`shard.rs:101`)
- `Active.file_id: u32` (`shard.rs:115`)
- `ImmutableFile.file_id: u32` (`shard.rs:124`)
- `BlockKey.file_id: u32` (`cache.rs:15`)
- `Shard::read_block(file_id: u32, ...)` (`shard.rs:405`)
- `Shard::active_file_id() -> u32` (`shard.rs:588`)
- `recovery.rs` parses filenames as `u32` (`recovery.rs:1598`)

The mismatch is bridged by ~23 `as u16` casts on the way into `DiskLoc::new`
and a handful of `loc.file_id as u32` casts on the way back out
(`shard.rs:367, 376, 421, 431`). `Shard::next_file_id` increments monotonically
on every file rotation (`shard.rs:823-824`) — it is **never** reused, even
after compaction deletes the old files. Once it crosses 65535, every new
`DiskLoc::new(new_id as u16, _, _)` silently aliases an older file ID. The
collision lands in two places:

1. **`Shard::read_block`** compares against the aliased `loc.file_id as u32`
   and may return data from the wrong file (if the alias hits a still-live
   immutable) or `StaleDiskLoc` (if the alias hits a removed file).
2. **`BlockCache`** keys are `(shard_id, file_id, block_offset)`. With
   `file_id` u16-aliased on the way into the index but `file_id` u32 on the
   way into `BlockKey`, the cache and the index disagree about which block
   they are talking about. Silent data corruption.

`max_file_size` is hard-capped at 4 GiB (`config.rs:94`) because `offset` is
`u32`. The default release config is `max_file_size = 256 MiB`, so the
file-id ceiling translates to:

| Scenario                     | Per-shard ceiling           | 16 shards    |
| ---------------------------- | --------------------------- | ------------ |
| Default (256 MiB per file)   | 65536 × 256 MiB = **16 TiB** lifetime writes | **256 TiB** |
| Maximum (4 GiB per file)     | 65536 × 4 GiB = **256 TiB** lifetime writes  | **4 PiB**    |

These are *lifetime byte counts* — compaction freeing space does not return
file IDs to the pool. For long-running write-heavy workloads (logs, append-
heavy stores) with small `max_file_size` for fast compaction, the ceiling is
within reach.

The fix is to widen `DiskLoc.file_id` to `u32` so it matches the rest of the
codebase end to end.

## Goals

1. `DiskLoc.file_id` becomes `u32`. After this change there is **no** lossy
   `u16 ↔ u32` conversion on either the index-write or index-read path.
2. The full file-rotation lifetime of a shard is bounded by `u32::MAX`
   (~4.29 billion), not 65535.
3. `Shard::read_block` and `BlockCache` see the exact same `file_id` value
   that was written into the index — no aliasing.
4. Recovery, compaction, and replication keep working without an on-disk
   migration. `DiskLoc` lives in RAM-only structures (SkipList nodes,
   per-shard `HashMap<K, DiskLoc>` and `HashMap<K, MapEntry<V, DiskLoc>>`);
   `file_id` is reconstructed from filenames (`{:06}.data`) at recovery.

## Non-Goals

- Reusing `file_id` after compaction. Out of scope — would require a
  generation/epoch counter to remain safe and we deliberately chose
  widening over reuse (see conversation 2026-05-17).
- Changing `max_file_size` cap or `offset` width.
- Replacing `{:06}` with `{:010}` in filename formatting. Padding is
  cosmetic; `parse::<u32>()` already handles any width. Leave existing
  files compatible. Optional cleanup deferred.
- A monotonic `file_id` hard-cap or panic near `u32::MAX`. After widening,
  the practical ceiling is so far out (4.29 B rotations × 4 KiB minimum
  file = 17.6 TiB minimum lifetime writes per shard) that adding a runtime
  guard is YAGNI.
- Reducing in-memory footprint of `DiskLoc`. The new layout is 4 bytes
  wider per index entry; we accept this cost.

## Design

### `DiskLoc` struct

**Before** (`disk_loc.rs:1-23`, `#[repr(C)]`, 12 bytes):

```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[repr(C)]
pub struct DiskLoc {
    pub offset: u32,
    pub len: u32,
    pub file_id: u16,
    pub shard_id: u8,
    _pad: u8,
}

impl DiskLoc {
    pub fn new(shard_id: u8, file_id: u16, offset: u32, len: u32) -> Self { ... }
}
```

**After** (16 bytes, 4-byte aligned):

```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[repr(C)]
pub struct DiskLoc {
    pub offset: u32,
    pub len: u32,
    pub file_id: u32,
    pub shard_id: u8,
    _pad: [u8; 3],
}

impl DiskLoc {
    pub fn new(shard_id: u8, file_id: u32, offset: u32, len: u32) -> Self { ... }
}
```

`_pad` widens from `u8` to `[u8; 3]` so natural alignment is preserved and
`size_of::<DiskLoc>() == 16` (verified with a unit test). The padding bytes
remain zero-initialized in `new`.

### Call-site updates

Drop every `as u16` cast that targets `DiskLoc::new` or `loc.file_id`. The
exhaustive list (from inventory):

**`as u16` casts to remove** (19 sites — all inputs into `DiskLoc::new`):

| File              | Lines                                                                 |
| ----------------- | --------------------------------------------------------------------- |
| `recovery.rs`     | 437, 494, 547, 646, 757, 858, 1116, 1202, 1238, 1321, 1471, 1565       |
| `var_tree.rs`     | 1118                                                                  |
| `var_map.rs`      | 754                                                                   |
| `const_tree.rs`   | 969                                                                   |
| `const_map.rs`    | 728                                                                   |
| `compaction.rs`   | 191, 217                                                              |
| `shard.rs`        | 792                                                                   |

**`as u32` casts to remove** — once `file_id` is `u32`, every
`loc.file_id as u32` becomes an identity cast and trips
`clippy::unnecessary_cast` under `-D warnings`. The exhaustive sweep
(`grep -rn 'file_id as u32' armdb/src/`) yields **53 sites across 7 files**:

| File              | Sites                                                                 |
| ----------------- | --------------------------------------------------------------------- |
| `var_tree.rs`     | 20 sites (incl. one test-only at line ~1648)                          |
| `var_map.rs`      | 17 sites                                                              |
| `typed_tree.rs`   | 5 sites                                                               |
| `typed_map.rs`    | 4 sites                                                               |
| `shard.rs`        | 3 sites (367, 376, 963)                                               |
| `compaction.rs`   | 2 sites (303, 331)                                                    |
| `durability.rs`   | 2 sites                                                               |

**Rule:** every `<binding>.file_id as u32` where the binding is a `DiskLoc`
(or a `DiskLoc` field destructure) must be dropped. Use the grep above as
the authoritative checklist during implementation; line numbers will shift
as files change between tasks.

After removal: `inner.active.file_id == loc.file_id`,
`f.file_id == loc.file_id`, `read_block(loc.file_id, ...)`, etc.

**Casts that stay:** the `as u32` casts at `shard.rs:421, 431` take a
`file_id: u32` parameter — those are not `DiskLoc`-derived and stay
unchanged.

**Signature updates:**

- `DiskLoc::new(file_id: u32, offset: u32, len: u32)`.

No public API outside `DiskLoc::new` exposes `file_id: u16` directly; the
field is `pub` and accessed by struct field, so widening the field type is
sufficient for all read sites.

### Filename format

`{:06}.data` (`shard.rs:209, 267, 294, 830`, `compaction.rs:105, 129, 263, 363`,
`recovery.rs:66, 120, 225, 330, 916, 1017, 1368`, `replication/log_reader.rs:264`)
is left as-is. Once `file_id > 999_999` the padding stops applying but the
generated names (e.g., `1000000.data`) are still valid filenames and
`parse::<u32>()` (`recovery.rs:1598`, `hint.rs:180`, `shard.rs:194`,
`replication/log_reader.rs:352`) accepts them.

A wider format (`{:010}`) is purely cosmetic for files past the million-
rotation mark and would create an existing-on-disk discontinuity, so we skip
it.

### No on-disk migration

`DiskLoc` is **never** serialized to disk. Verified:

- Hint files contain `(GSN, Key, Offset, Len)` — no `file_id`
  (`hint.rs:103-107`, format `GSN(8) | Key | Offset(8) | Len(4)`).
- Bitcask segment headers/footers and FixedStore meta do not store
  `DiskLoc`; recovery rebuilds in-memory indexes by scanning files.
- Replication wire format passes raw entries with `shard_id`; `file_id` is
  assigned locally on the replica during apply (no `DiskLoc` on the wire,
  `replication/protocol.rs`, `fixed_replication/protocol.rs`).
- Snapshots: same — they capture the live entry stream, not raw `DiskLoc`s.

An existing database on disk has no `file_id` values stored anywhere; they
are derived from filenames on the next open. Pre-upgrade databases load
cleanly; post-upgrade databases write `file_id` values that fit in the new
`u32` field (and trivially in the old `u16` for the first 65536 rotations).

### In-memory footprint

| Collection                                              | Per-entry before | After     | Delta |
| ------------------------------------------------------- | ---------------- | --------- | ----- |
| `HashMap<K, DiskLoc>` (VarMap, ConstMap value-only)     | `sizeof(K) + 12` | `+ 16`    | +4    |
| `HashMap<K, MapEntry<V, DiskLoc>>` (ConstMap with V)    | `+V + 12`        | `+V + 16` | +4    |
| `SkipList VarNode { disk: AtomicPtr<DiskLoc>, ... }`    | 12-byte heap box | 16-byte   | +4    |
| `BlockKey { shard_id, file_id, block_offset }`          | unchanged (already u32) | unchanged | 0 |

For 100M live entries, the RAM cost is ~400 MiB. Documented as the
trade-off in PR description.

## Tests

All new tests live alongside existing tests in the affected modules.

### `disk_loc.rs` — struct invariants

- `disk_loc_size_is_16` — `assert_eq!(size_of::<DiskLoc>(), 16)`.
- `disk_loc_alignment_is_4` — `assert_eq!(align_of::<DiskLoc>(), 4)`.
- `disk_loc_new_zeroes_padding` — assert the `_pad` bytes after `new(...)`
  are zero, so `Eq`/`Hash` derived semantics stay deterministic.
- `disk_loc_roundtrip_large_file_id` — construct with
  `file_id = u32::MAX`, read the field back, compare; same for `65536`,
  `65537`, `1_000_000`.

### `shard.rs` — read path with large file_id

- `read_block_with_file_id_above_u16` — open a shard, manually rotate the
  active file until `inner.active.file_id > 65535` (use the test helper that
  bumps `next_file_id`), write an entry, capture its `DiskLoc`, read it back
  through `Shard::read_block` and compare bytes. Without the widening this
  test reads from the aliased lower-16-bit file (or fails to find it).
- `write_buffer_read_with_file_id_above_u16` — same, but read from the
  active write buffer (before rotation).

To avoid a 65536-iteration loop, add (or use, if it already exists) a
`#[cfg(test)] fn set_next_file_id(&mut self, id: u32)` helper that bumps the
counter directly.

### `var_tree.rs` / `var_map.rs` — index round-trip

- `var_tree_get_with_file_id_above_u16` — put a value into a shard whose
  `next_file_id` was bumped past 65535, force rotation so the entry lands on
  an immutable file with `file_id > 65535`, `get` returns the correct bytes.
- `var_map_get_with_file_id_above_u16` — same shape for `VarMap`.

Geometry mirrors the existing large-value tests:
`shard_count = 1`, `write_buffer_size = 128 KiB`, `max_file_size = 4096`
to force rotation per entry.

### `cache.rs` — BlockKey no-aliasing

- `block_cache_disambiguates_file_ids_across_u16_boundary` — insert two
  blocks at the same `(shard_id, block_offset)` with `file_id = 65535` and
  `file_id = 65536`, confirm both are independently retrievable. Without
  the widening these would alias to the same `BlockKey` in the index path
  (and the test would silently overwrite one).

### `recovery.rs` — recovery roundtrip

- `recovery_handles_file_id_above_u16` — write a DB, simulate a shard whose
  files include `0000001.data` … plus a synthetically renamed
  `1000000.data` (within `u32` range, past `u16`), open the DB, confirm
  recovery succeeds and entries from the wide-id file are reachable through
  `get`.

### Existing test sweep

`cargo nextest run -p armdb --features var-collections` and the encryption
feature must all stay green. Existing tests use `file_id` values well under
65535 and continue to exercise the common path.

## Verification

```sh
cargo check -p armdb
cargo check -p armdb --features var-collections
cargo check -p armdb --features 'var-collections encryption'
cargo nextest run -p armdb
cargo nextest run -p armdb --features var-collections
cargo nextest run -p armdb --features 'var-collections encryption'
cargo clippy -p armdb --features var-collections -- -D warnings
cargo fmt --check
```

## Files Changed

- `armdb/src/disk_loc.rs` — widen `file_id` field, widen `new` signature,
  widen `_pad` to `[u8; 3]`, add struct-invariant unit tests.
- `armdb/src/shard.rs` — drop `as u16` at 792, drop `as u32` at 367/376/963,
  add `set_next_file_id` test helper, add read-path tests.
- `armdb/src/compaction.rs` — drop `as u16` at 191/217, drop `as u32` at
  303/331.
- `armdb/src/recovery.rs` — drop the 12 `as u16` casts.
- `armdb/src/var_tree.rs` — drop `as u16` at 1118, drop all 20 `as u32`
  identity casts, add e2e test.
- `armdb/src/var_map.rs` — drop `as u16` at 754, drop all 17 `as u32`
  identity casts, add e2e test.
- `armdb/src/typed_tree.rs` — drop all 5 `as u32` identity casts.
- `armdb/src/typed_map.rs` — drop all 4 `as u32` identity casts.
- `armdb/src/durability.rs` — drop all 2 `as u32` identity casts.
- `armdb/src/const_tree.rs` — drop the `as u16` cast at 969.
- `armdb/src/const_map.rs` — drop the `as u16` cast at 728.
- `armdb/src/cache.rs` — add BlockKey disambiguation test.

## Out of Scope (explicit)

- `file_id` reuse pool.
- Generation/epoch counters.
- `{:06}` → `{:010}` filename padding.
- On-disk migration (none required).
- Replication protocol version bump (no wire change).
- Reducing memory footprint elsewhere to offset the +4 bytes per entry.

## Follow-Up

If profiling later shows the +4 bytes per index entry matters for
memory-constrained deployments, options include:

1. Packing `file_id` into a `u24`-style triple with `shard_id` (saves 1 byte
   but costs bit-fiddling on every read).
2. A separate, smaller `CompactDiskLoc` for collections with known small
   file-id ranges (premature without numbers).

Tracked separately.
