Skip to content

fix(storage): quarantine bloated HNSW link payloads - #1339

Merged
igorls merged 1 commit into
MemPalace:developfrom
fatkobra:fix/1218-hnsw-link-payload-health
May 7, 2026
Merged

fix(storage): quarantine bloated HNSW link payloads#1339
igorls merged 1 commit into
MemPalace:developfrom
fatkobra:fix/1218-hnsw-link-payload-health

Conversation

@fatkobra

@fatkobra fatkobra commented May 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #1218.

This extends the pre-open HNSW health gate so MemPalace quarantines structurally corrupt HNSW segments where link_lists.bin is wildly larger than data_level0.bin, even when index_metadata.pickle looks valid.

What changed

  • Added a structural HNSW payload sanity check:
    • compute link_lists.bin / data_level0.bin
    • treat ratios above 10x as corrupt
  • Updated _segment_appears_healthy() so a valid pickle alone is no longer enough to mark a segment healthy.
  • Updated quarantine_stale_hnsw() so link-payload corruption is not blocked by the mtime gate.
    • stale-by-mtime + bad pickle is still handled as before
    • extreme link/data bloat is quarantined immediately
  • Added regression tests for:
    • ratio calculation
    • rejecting exploded link_lists.bin with a valid pickle
    • keeping reasonable payload ratios
    • quarantining link bloat even when sqlite/HNSW mtimes are identical
    • leaving healthy segments untouched

Why

The latest #1218 investigation found a false negative in the existing HNSW sniff-test:

  • index_metadata.pickle was structurally valid
  • link_lists.bin had exploded relative to data_level0.bin
  • Chroma/HNSW then segfaulted on official tooling paths such as status/repair/migrate

The old gate answered only “does the pickle look complete?” This PR also asks “does the HNSW payload next to it look physically sane?”

That matters because ChromaBackend._prepare_palace_for_open() runs quarantine_stale_hnsw() before constructing a Chroma client. By catching the bloated payload there, MemPalace can rename the unsafe segment before Chroma's native HNSW reader has a chance to crash the process.

Threshold

The threshold is intentionally conservative:

link_lists.bin / data_level0.bin > 10x

## How to test
ruff format mempalace/backends/chroma.py tests/test_hnsw_payload_health.py
ruff check mempalace/backends/chroma.py tests/test_hnsw_payload_health.py
python -m pytest tests/test_hnsw_payload_health.py -q
python -m pytest tests/test_hnsw_payload_health.py tests/test_repair.py -q
python -m pytest tests/ -q

## Checklist
- [x] Tests pass (`python -m pytest tests/ -v`)
- [x] No hardcoded paths
- [x] Linter passes (`ruff check .`)

@moonn1000

Copy link
Copy Markdown

Threshold 10× checks out empirically against the data from #1218: the crashed palace ran at ~314 (24 GB / 76 MB), healthy snapshots stayed ~0.005 (88 KB / 17.4 MB at 49K drawers). Four orders of magnitude between corruption and any plausible healthy ratio I've seen.

The bypass of the mtime gate when payload_corrupt is the right call — my crashed segment had data_level0.bin mtime aligned with chroma.sqlite3 (write was still active when the bloat hit), so an mtime-only gate would have skipped it. Good to see test_quarantine_catches_link_bloat_without_mtime_drift covering that.

If a reproducer fixture would help, I still have the corrupted palace (~6 GB compressed, link_lists.bin is mostly zero-padded). Happy to upload to a release asset, push via git-lfs, or any S3 link the maintainers prefer — let me know what works.

@fatkobra

fatkobra commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @moonn1000 for validating the threshold and for the detailed data — super helpful.

Good to know the mtime bypass matches the real failure case you saw.

And yeah, a reproducer could definitely be useful for future regression testing — I’d be in favor of including it if maintainers are open to it.

@moonn1000

Copy link
Copy Markdown

@fatkobra @jphein — quick re-ping on the reproducer offer.

Tarball is still on hand for the corrupted palace from #1218 (the link_lists.bin = 24 GB / data_level0.bin = 76 MB / ratio ~314 case). Bundle would contain:

  • chroma.sqlite3 — intact, 302 MB
  • <segment-uuid>/ — full corrupted segment dir (including the 24 GB zero-padded link_lists.bin)
  • A small README pinning chromadb 1.5.7 + mempalace 3.3.3 + drawer count (~85 K)

Compressed size ~6 GB (the link_lists payload is mostly zero-padded so it compresses well).

Just need a destination preference from a maintainer:

  • GitHub release asset on the repo (simplest, but >2 GB single-file limit on standard releases — would need to split or use LFS)
  • git-lfs in a side branch
  • Any S3/CDN link the project would prefer me to upload to

Happy to also strip it down further (e.g., truncate link_lists.bin to the first few MB if a smaller pathological-ratio reproducer is enough for the regression test — the corruption shape, not the absolute size, is what _segment_appears_healthy() keys on after this PR). Let me know which form is most useful.

Also worth knowing: I just hit a separate repair --yes failure mode on a different palace (SQLITE_CORRUPT_INDEX at delete_collection before rebuild — written up at #1218). That one's a much smaller reproducer (~286 MB) and a different failure surface from this PR's scope, but if a fixture for SQLite-side corruption-vs-HNSW-side corruption discrimination would be useful in a follow-up, I have it too.

@igorls igorls added bug Something isn't working storage labels May 6, 2026
@igorls igorls added this to the v3.3.5 milestone May 7, 2026
@igorls
igorls merged commit 88a2ebb into MemPalace:develop May 7, 2026
6 checks passed
igorls added a commit to fatkobra/mempalace that referenced this pull request May 7, 2026
Two conflicts, both because MemPalace#1339 (bloated link payloads) merged into
develop after this branch was authored:

- mempalace/backends/chroma.py: _segment_appears_healthy now stacks
  three checks — bloated-link from MemPalace#1339 (top), missing-metadata-with-
  data-floor from this branch (middle), pickle format sniff (bottom).
  All three are complementary; MemPalace#1339 catches structural payload
  corruption, this branch catches pickle truncation, the original
  catches pickle protocol-byte corruption.

- tests/test_backends.py: kept both new imports (_segment_appears_healthy
  from this branch, quarantine_invalid_hnsw_metadata from MemPalace#1285).

Local: 1618 tests pass, ruff lint+format clean against 0.4.x CI pin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mempalace migrate/status crash with SIGSEGV on chromadb version mismatch — palace unrecoverable via official tooling

3 participants