Repository navigation
perf(api): run chunk-API initial builds on the bulk builder (#1901) - #1924
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
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 |
8ea57a8 to
42127d7
Compare
…1901) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
42127d7 to
df99dcc
Compare
Closes #1901
Slice of #1881. Initial builds through the chunk API (
GraphConstructionSessionappend_nodes/append_edges) now run on the bulk builder.What changed
accepted_chunksis "durably accepted",resume_graph_constructionreopens, exact replay by chunk id is idempotent, the crash matrix kills the append at every boundary). So each accepted chunk is spooled once as one self-describing Arrow IPC file: temp name, synced, renamed, directory synced. The footer carries the chunk id, kind, sequence, row count and digests, so there is no receipt journal, chunk key or per-chunk checkpoint rewrite.appendcall with the same messages; global refusals (duplicates, endpoints) fire at seal.BulkBuildPlan::routeover the receipts. Only a build whose node tables alone exceed the budget replays the spool through the staged path, the same case an import session stages. The route is recorded before any work and read back on retry.spooled chunk differs from its acknowledged digest.List<Utf8>values["a, b", "c"]and["a", "b, c"]digested alike. It is now a typed, recursive, length-prefixed encoding with validity markers (hash_column). This changesinput_sha256for property-bearing chunks on the staged path too (pre-v1, in place).StableDirectory::open_child_file. Import sessions stage through the newbegin_staged_graph_construction; the facade'sbegin_graph_constructionspools on an empty project. Appends to a non-empty graph stage.resumable-import.mdare updated; the Rust, Python and Node release-surface parity contracts record the new public method.Review findings (#1924 at 8ea57a8)
the_chunk_digest_separates_values_that_print_alikeand thelist property that prints alikecorruption case (same-size edit turning["a, b","c"]into["a","b, c"]). With the list rows hashed by display text again, both fail ("corruptions not refused: list property that prints alike").a_chunk_admitted_at_its_exact_byte_window_replays_through_the_staged_path. Re-measuring on replay fails with "construction resource window exhausted"; reverting the fix reproduces it.appendreturns, so no crash leaves an acknowledged chunk missing, truncated or half-written; only an unacknowledged temporary file can remain, and reopen removes it. The reviewer's scenario needs someone to edit or delete files inside the private session directory and rewrite the footers, which is outside the threat model as for the staged artifacts. A hole in the sequence is still refused ("not contiguous"). Documented in thespool.rsmodule docs; the kill-at-every-failpoint tests (a_process_killed_while_accepting_chunks_resumes_and_continues,a_kill_between_rename_and_directory_sync_loses_nothing_acknowledged) pass. A process kill cannot lose page-cache data, so no test can show the barrier order by killing; it is argued from the code.StableDirectoryuse. Added: chunk API equals registered sources (spooled_chunks_publish_the_bytes_of_registered_sources).node labelis missed); Utf8 property values not hashed (onlyproperty valueand the list case are missed); the logical-digest check disabled for node chunks, then for edge chunks (the node identity and edge endpoint cases are missed respectively).Acceptance criteria
chunk_api_initial_builds_take_the_bulk_path,an_over_budget_chunk_api_build_runs_on_scratch_without_staging, storagean_over_budget_spooled_build_takes_the_scratch_route_and_publishes_the_staged_bytes.cmpdiffers, binary sha256 basea2ec3ccf...c728a, candidate19bc2514...90da4. Full table and commands on perf(api): run chunk-API initial builds on the bulk builder #1901.captured encoded source identity or length changed(unsynced artifactst_blockschanges under ext4 delayed allocation; 5 of 6 runs of an import session on main, 3 of 7 of the chunk-API candidate). Not caused or fixed here.Verification (head df99dcc, rebased on main after #1910, #1923, #1926, #1927)
cargo nextest run -p graphforge-storagewithGF_TEST_TMPDIR=/home/ubuntu/t1901: 1845 run, 1845 passed (includes the spool authentication, digest, replay-admission, scratch-route and chunk-versus-registered-source equivalence tests).cargo nextest run -p graphforge-api -E 'not binary(bdd)'in a non-nested worktree: 1702 run, 1702 passed.cargo test -p graphforge-api --test bdd: 118 passed.make check: exit 0.🤖 Generated with Claude Code