fix(storage): stop the collection metadata chain from growing with every save (#195)

[CaffeinatedCoder]

Fixes #195.

SaveCollectionMetadata no longer adds a record to the metadata chain on every save. A chain that has already grown shrinks back on its next metadata save. Inserts into int/long-keyed collections, which save metadata once per insert through GetNextSequence, now cost the same at the 30,000th insert as at the first.

Changes

All in StorageEngine.Collections.cs:

1. In-place rewrite. A record that is no larger than the stored one is overwritten where it is, in one page write. This is the common case: a sequence value or a root page id changes, and the record keeps its size.

2. Compaction before appending. For a record that grew, or a new one, CompactMetadataChain first:

• packs the live records from page 1 on, keeping their order;
• drops deleted slots, the bytes left behind by records overwritten with shorter ones, and duplicate copies of a record (it keeps the first, as lookups do);
• frees the pages that are no longer needed.

Then the record goes into a page with room, or into a new page linked at the end.

3. Write order. The new record is written before the old one is marked deleted, so a crash in between leaves the old record instead of neither. Before, the deletion was written first.

4. Healing. After an in-place write, the chain is compacted if that frees at least one page. A database grown by earlier releases shrinks on its first metadata save after the upgrade: opening a collection, inserting with a generated int id, creating an index, and so on.

5. Crash safety of compaction.

• Pages are rewritten front to back, and greedy packing in order puts every record on its own page or an earlier one. So after each page write, every record is still on some page.
• A crash part-way can leave a record on two pages. The copies are identical and readers take the first one.
• To keep that harmless, GetAllCollectionsMetadata returns each collection once, DeleteCollectionMetadata deletes every copy (before, it stopped at the first), and the next compaction drops the extra copy.

Freed pages go back to the free list, so the next allocation reuses them.

Measurements

The probe from #195, on the same machine, run on this branch and on main:

GetNextSequence calls so far	chain pages: main	per call: main	chain pages: this PR	per call: this PR
1,000	3	0.10 ms	1	0.09 ms
11,000	26	0.26 ms	1	0.05 ms
51,000	119	1.00 ms	1	0.06 ms
101,000	235	1.94 ms	1	0.06 ms

The file stayed at 2 MB on this branch; on main it grew from 3 MB to 6 MB.

Typed inserts into TestDbContext.Products (int id left at 0):

inserts	main	this PR
0 to 5,000	195 µs	128 µs
10,000 to 15,000	430 µs	121 µs
25,000 to 30,000	958 µs	117 µs

Tests

MetadataChainTests, 6 tests, all failing on main:

• Sequence saves: 5,000 GetNextSequence calls leave the chain unchanged with no deleted slots, and the value survives a reopen.
• Typed inserts: 2,000 inserts with generated int ids give ids 1..2000 and a one-page chain.
• Records that shrink and grow: 500 alternating saves of a record growing and shrinking by about 400 bytes keep the chain at its length, and the last values are read back.
• A chain of several pages:
  • 40 records of about 1.2 KB fill exactly the minimal number of pages;
  • 20 rounds of rewrites keep that length;
  • after 30 collections are dropped, the next save packs the rest into page 1 and frees the other pages, which the next allocation reuses;
  • everything reads back after a reopen, and documents are still found.
• Chains grown by older releases: a test helper recreates the old save layout byte for byte, with 3,000 saves spread over at least 7 pages. The collection still reads correctly. The next metadata save (creating an index) shrinks the chain to page 1, keeps all 8 collections, and frees the old pages for reuse.
• Duplicates after a crash: a duplicate record placed on a later page is listed once, is dropped by the next compaction, and is removed together with the original by DeleteCollectionMetadata.

I disabled each part in turn and checked that at least one test fails each time: the in-place path, the healing compaction, the compaction before appending, freeing the surplus pages, deleting every copy, listing once, and dropping duplicates. The write order (new record before deletion) is the one part not covered by a test, since it would need a crash between two page writes.

Full suite on macOS: 2398 passed, 0 failed, 4 skipped. BLite.NetStandard21.Tests: 20/20. BLite.Core builds with no warnings.

Merging with #194

#194 changes the same method, so the second of the two to land needs a small conflict resolution:

• move feat(storage): per-collection required features, refused by versions that lack them (#187) #194's guard to right after the record is located, so it also covers the in-place path (if (foundPage >= 0 && TryReadRecord(chain[foundPage].Buffer, foundSlot) is { } stored) CollectionFeatures.EnsureCanWrite(stored););
• keep both TryReadRecord and TryReadRecordName.

I resolved it on a scratch merge: the tests of both PRs pass (17/17), and the full suite gives 2409 passed, 0 failed, 4 skipped. Compaction moves the records of collections with unknown features without changing a byte, which keeps #194's guarantee. I'll push the resolution to whichever branch lands second. The other open PRs merge with this one without conflicts.

Not changed here

• Unlocked readers. GetCollectionMetadata and GetAllCollectionsMetadata still read without the metadata lock, as before. A reader running at the same moment as a save that moves records could miss one. The old save had the same window between writing the deletion and writing the new record. The common in-place path now leaves a much smaller window: one page write.
• Multi-process. The metadata lock is per process, so two processes can still save metadata concurrently (see Multi-process mode: two processes opening a new model both backfill the same index #192).

🤖 Generated with Claude Code

[mrdevrobot]

Thanks. I reproduced #195 on main (all 6 new tests fail there and with the fix reverted). Probe on the merged state: 10,000 alternating 600-byte / 5-byte saves plus 10,000 GetNextSequence calls keep the chain at one page; on main it ends at 517 pages. I also checked crash safety by splicing file images after each page write of a 4-page compaction with holes: every intermediate state read back all 26 surviving collections correctly, and the write order (new page before its link, new record before the old one's deletion) is better than before. The format is unchanged and 5.2.x reads the result. Metadata pages never go through the WAL, so there's no interaction with the deferred frees from #169. Full suite merged with main 2463 / 0 / 4 skipped.

Approving. One follow-up I'd like soon, here or right after:

Unlocked readers can miss a collection that isn't being saved (GetCollectionMetadata ~L120, GetAllCollectionsMetadata). A compaction moves collection B's record from page p+1 to p and frees the tail. A reader walking for B without the lock can read page p before the rewrite and p+1 after it, or reach a freed page, and return null. The DynamicCollection and CollectionIndexManager constructors treat null as "new collection" and save fresh metadata, which overwrites B's root page ids and orphans its data. Before, only the collection being saved could be missed, so "the old save had the same window" isn't quite right. Fix: when the walk returns null or finds a duplicate, walk again under _metadataLock.

Smaller: CompactMetadataChain(onlyIfShorter: true) copies every live record on every in-place save, including each GetNextSequence, just to count pages; counting from slot lengths first avoids that. A crash between the last compaction write and the FreePage loop leaks the tail pages (acceptable, worth a comment). A test for the new up-front size check would be good. Separately, repeated index create/drop still grows the file on main for a different reason: #209.

On merge order with #194, see my note there: the feature guard has to move before your in-place step.

[CaffeinatedCoder]

Thanks. The follow-up is in #215. It is stacked on this branch, so this PR can merge as approved.

• GetCollectionMetadata: if the lock-free walk misses the record or hits a page it can't parse, it walks again under _metadataLock. A record it finds is returned as before.
• GetAllCollectionsMetadata: walks under the lock. For the full listing, null or a duplicate isn't a usable signal: records moved behind the reader just leave the result short.
• CompactMetadataChain(onlyIfShorter: true): returns at once for a one-page chain. Otherwise it plans from slot lengths and copies only when it rewrites. The tail-page leak on a crash before FreePage is now documented.
• New tests:
  • the reader/compaction race, made deterministic for both readers; the tests fail on this branch as it stands;
  • the up-front size check. Without it, a record one byte too large is accepted silently, its slot entry overlapping the record's first byte, and the old record is then deleted.
• GetNextSequence stays at ~0.06–0.07 ms on a one-page chain (0.08 vs 0.095 ms on a three-page chain).

#209 is left for its own fix.