Skip to content

Avoid per-chunk cache writes for absent indexes - #592

Closed
forhappy wants to merge 1 commit into
codex/catalog-current-integrationfrom
codex/add-classifier-io
Closed

forhappy wants to merge 1 commit into
codex/catalog-current-integrationfrom
codex/add-classifier-io

Conversation

@forhappy

@forhappy forhappy commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

On a fresh protocol-v2 bucket, native crab add opens the optional legacy global chunk index and correctly remembers that it is uninitialized. The classifier nevertheless wrote one advisory negative SQLite cache row per chunk. An actual 64 MiB add produced 1,037 such rows despite the index being absent before and after the command.

Persist candidate-cache updates only after the shared index has actually opened. Keep absence of the entire index in the existing command-scoped OnceCell. Initialized index snapshots keep their current positive/negative cache behavior. Candidate selection, local payload ownership, cancellation, explicit close and push-side proof validation are unchanged.

Draft, stacked on #587 (codex/catalog-current-integration). Separate from the browse-retention changes in #591. No merge requested.

Validation

  • Focused classifier and candidate-cache tests: 16 passed, including absence remembered across distinct batches without persistent negative writes, an initialized-index control that still persists misses, and cancellation/close behavior.
  • The test executable built successfully with the existing minimal simd-accel,gix-pathmatch feature slice. Existing unrelated feature warnings remain.
  • Format and diff checks: passed. No dependency, configuration, public API or data-format changes.
  • Real CLI baseline: fresh owned RustFS bucket, deterministic 64 MiB file, actual native add and Git-index pointer validation; legacy global index absent; 1,037 negative cache rows. The classifier source is identical to this PR's parent. The raw baseline report also records generated Python bytecode in its harness source tree, so it is diagnostic evidence rather than accepted clean-source release qualification.
  • Combined-source fixed CLI verification now passes as detailed below. The standalone f3c9712 binary has not been separately reinstalled. Combined 10 GiB multi-edit/push qualification remains running; no end-to-end latency improvement is claimed.
CARGO_TARGET_DIR=<checkout-specific mounted-volume directory> cargo test -p crab --locked --no-default-features --features simd-accel,gix-pathmatch --lib -- add_classifier_ cache::add_remote_candidates

Review evidence

Boundary Evidence
Entry/owner Native add constructs AddRemoteChunkClassifier; its existing ExistingChunkLookup implementation owns advisory candidate-cache writes
Callee Read-only metadata guard opens the global chunk index; structured uninitialized errors initialize the session cell to None; other errors propagate
Sibling Git clean uses the same staging byte engine with local-only lookup, so it never invokes this classifier/cache path
Push contract Positive placements still cross normal placement/origin proof validation at push; cache misses retain local payload authority
Current main Final main integration proof is pending; this draft targets the unmerged #587 stack and validates its classifier implementation
Parent behavior Index absence is already probed once, but every fetched miss is still persisted as a per-chunk negative
Tests Absence, initialized-index control, TTL/promotion/capacity/concurrency, cancellation and explicit close

Is this the best fix? Cache the fact at the scope where it is known: absence of the whole index belongs to the current lookup session. Removing these writes requires no schema migration or new option and preserves the initialized-index contract.

This does not integrate native add with the v2 catalog or solve the roughly 10 GiB of local payload writes still observed for small edits. Those remain separate staging work. The two excluded large-file audit reports and the user-owned advisor plan are not included.

Live integrated verification (909a40a2559):

  • The official frozen combined bundle passed 20 commands / 34 checks on a fresh owned RustFS/v2 origin. Native add processed 64 MiB, then processed a second 64 MiB version with a deterministic 1 MiB edit. Both commands staged a real Git-index pointer and preserved the independently seeded working bytes; the pointer identities differ.
  • The uninitialized legacy chunk-index prefix remains empty before and after each add. Before any push could clean cache entries, direct read-only SQLite inspection found zero positive rows, zero negative rows and matching zero cached counts after both 1,064- and 1,060-chunk passes. Remote lookup timing was nonzero in both passes.
  • An independent acceptance audit checked the raw add summaries, seed digests, command history with no push, and the pointer/version records. The classifier and advisory-cache source blobs in the combined build are byte-identical to this PR.
  • This verifies the integrated absent-index write behavior only. It does not initialize a v1 index, synthesize v1 placement/proof IDs, prove the final large-file matrix, or solve incremental local payload ownership.

An unopened global index cannot supply per-chunk absence facts. Keep that state in the existing OnceCell instead of persisting a negative row for every chunk in a new file.

Preserve initialized-index cache policy, cancellation, close ownership and push-side proof validation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant