Repository navigation
Conversation
A loader with N edges sent one GRAPH INSERT EDGE or GRAPH DELETE EDGE per edge. Each cost a round trip and its own Calvin transaction. GRAPH INSERT EDGES and GRAPH DELETE EDGES now carry the edges of one statement in a VALUES list. The handler builds one task with every edge and submits it as one sequenced Calvin transaction. The submit path already slices a batch per home pair and sums the owned-edge counts (edge_slices, ReplyFold::SumOwnedEdges). Every edge lands or none does. A deadline that ends the wait leaves an unknown but all-or-nothing outcome, and a retry is idempotent. Surrogates for all endpoints of a statement are assigned with one assign_surrogates_routed call. Write policy is evaluated per edge with resolve_edge_write_rls. A delete under a write policy keeps the per-edge path: the allowed edges apply, the denied edge stays, and the statement returns an error. A batch inside a transaction block validates the whole list before it stages any tuple. The bounds are protocol constants, MAX_EDGES_PER_BATCH_STATEMENT and MAX_BATCH_TEXT_BYTES, so the parser and the client agree: 1000 edges or 1 MiB of SQL text, whichever the statement crosses first. The error names the limit.
`batch_graph_insert_edges` builds one `GRAPH INSERT EDGES` statement from the edge list. It sends the statement over the existing `Sql` opcode and checks the protocol bounds first. An oversized batch fails with the limit named, before any round trip. The trait default keeps the per-edge path for a client that cannot send the batch statement.
Wire tests pin the affected count, the per-statement bounds, and the delete count against the removed edges. They pin the per-edge write policy: a delete batch with an allowed and a denied edge removes the allowed edge and keeps the denied one, and an insert batch under a policy that needs a property refuses every edge. The random-id tests pin that 240 edges over many home pairs land completely, that a refused member lands nothing, that a deadline leaves either no edge or all of them, and that a retry after a delete batch is idempotent.
The parser caps a batch at MAX_EDGES_PER_BATCH_STATEMENT, but the handler sized its buffers from the edge count without repeating the cap. A hand-built AST skips the parser, so the allocation size came from the caller. CodeQL flagged both Vec::with_capacity calls in the resolve step. validate_edge_batch now refuses a list over the cap with SQLSTATE 54000 and names the limit, before any buffer is sized. Unit tests cover the cap, one edge over it, and the empty and collection-less shapes.
CodeQL flags Vec::with_capacity(edges.len()) in the insert and delete resolve functions as an allocation sized from user input. The parser and the handler cap the list at MAX_EDGES_PER_BATCH_STATEMENT, but the analysis does not follow a guard that lives in another function. Build the three buffers without a size hint. The list holds at most 1000 entries, so growth costs nothing measurable and no allocation size comes from the caller. Also correct the delete_edges doc: on the Calvin route each home counts the edges it owns and the answers add up.
BoundedEdges::new is the only way to make a checked edge batch. It checks the collection, the count (1000, SQLSTATE 54000), the endpoints and label, and removes duplicates. The resolve functions take BoundedEdges, so no caller can size a buffer from an unchecked count.
Contributor
Author
|
Two commits added: the batch resolve step no longer sizes a buffer from the edge count (CodeQL alerts at |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
GRAPH INSERT EDGESandGRAPH DELETE EDGES. One statement carries up to 1000 edges and runs as one sequenced Calvin transaction. Refs #369. It does not close #369 (see "Not in this PR").Changes
feat(graph)edge_batch.rs,edge_batch_resolve.rs). The handler builds one task with every edge and submits it withbuild_single_vshard_tx_class. Surrogates for all endpoints come from oneassign_surrogates_routedcall.feat(client)batch_graph_insert_edgesbuilds oneGRAPH INSERT EDGESstatement and checks the bounds before the round trip.test(graph)fix(graph)54000) before it sizes any buffer. CodeQL flagged bothVec::with_capacity(edges.len())calls, because a hand-built AST skips the parser. Three unit tests cover the cap.The submit path already slices a batch per home pair and sums the owned-edge counts (
slice_edge_batches,ReplyFold::SumOwnedEdges), andwrite_keys/graph.rsalready locks the keys ofEdgePutBatchandEdgeDeleteBatch. The handler only has to hand it one task.Semantics
statement_timeoutof 30 ms and 120 ms the client gotexceeded deadlineand all 240 edges had landed. At 1 us and 5 ms none had. No partial result was seen.MAX_EDGES_PER_BATCH_STATEMENT(1000) andMAX_BATCH_TEXT_BYTES(1 MiB). The parser and the client share them.resolve_edge_write_rls. An insert batch has no properties, so a policy that needs a property refuses every edge. A delete under a write policy keeps the per-edge path: the allowed edge goes, the denied edge stays, and the statement returns an error.Measurements
Single node, release build, 2 data-plane cores, trust mode, psycopg2 autocommit.
GRAPH INSERT EDGEGRAPH INSERT EDGESOn a production-size store (6.5 GB, 82k symbols) under host load, 1000 random-id edges took 15 to 18 s for insert and 14 to 16 s for delete. Inserts landed and deletes removed every sampled source. The same shape as single statements costs about 160 ms per edge.
Checks
cargo fmt --all -- --check: pass.cargo check -p nodedb --tests --locked: pass.cargo test --profile ci -p nodedb --test wire -- graph_dsl_batch_edges graph_batch_rls --test-threads=2: 20 pass.cargo test -p nodedb-sql graph: 82 pass, with 7 new batch parser tests.cargo test -p nodedb-client --all-features --lib graph_dsl: 34 pass, with 7 newinsert_edges_sql_*tests.cargo test -p nodedb --lib -- edge_batch::tests: 3 pass (the handler cap).nodedblibrary unit tests.Not in this PR
batch_graph_insert_edgeson the native client sends the statement text over the existingSqlopcode. NoOpCodeorGraphOprequest field exists for the batch, and the client has nobatch_graph_delete_edges. The issue asks for it.BatchEdgehas no write-gate slot. Fixing it needs a policy slot there or a pre-check of all members.request req:0 exceeded deadlineand does not say the outcome is unknown.Found while testing, not changed here
A graph-edge statement whose deadline expires inside
mark_collection_edge_bearingblocks the metadata propose path. The nextCREATE COLLECTIONthen fails after 60 s withmetadata propose: request req:0 exceeded deadline. It reproduces onmainwith oneGRAPH INSERT EDGEafterSET statement_timeout='1us'.