Repository navigation
perf(storage): hash CSR shards on write instead of reading them back (#1384) - #1552
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: CurateLabs/graphforge/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
The last remaining read-back-to-hash in the encode publish path.
write_csr_shardwrote each bounded CSR shard, then read the whole file back to hash it (codec::read+sha256_hex), doubling the shard's device I/O on the publish-side critical path. That violates #1384's accepted decision 1: no pass reads data back in order to hash it.Now each shard is encoded once into memory (shards are admission-bounded, ≤1M entries), the exact bytes that land on disk are preflight-verified and SHA-256'd, and the file is written once. The recorded digest is identical to what the read-back produced, so receipts, shard-set reuse and manifest naming are unchanged.
Refusals are preserved, not moved
codec::admit_encoded_lenapplies the sameencoded_limitbefore any byte reaches the filesystem instead of after the read-back.What remains read-back on purpose
Small JSON controls (per-index
.csr.json, adjacency manifest) are still verified by read-back inencode_adjacency— kilobytes against gigabytes, and an independent on-disk check of the shard digest chain. The 45 GBshape_consume_reauthenticationrow in the retained baseline is the whole shaping region under a legacy phase name (its reads are the partitioner's real data movement), not authentication; that accounting note feeds #1384's boundary ledger.Validation
TMPDIR=<ext4>cargo test -p graphforge-storage --lib: 1,277 passed, 0 failed (includes the adjacency determinism and portable-import publish paths; the 3 tests that fail under the default tmpfs/tmpfail identically on the pristine base — the documentedGF_UNSUPPORTED_FILESYSTEMadmission).cargo fmt --all -- --checkclean;cargo clippy --workspace -- -D warningspasses.cargo test -p graphforge-benchmark-certifycompiles clean against the changed API.Refs #1384
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.