Skip to content

feat(graph,obsv): batched edge writes + loader-facing observability - #374

Closed
EnRaiha wants to merge 5 commits into
mainfrom
feat/etl-batch-and-loader-observability
Closed

EnRaiha wants to merge 5 commits into
mainfrom
feat/etl-batch-and-loader-observability

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

What

Three commits closing the loader-facing gaps from the ETL port review:

  1. feat(graph): GRAPH INSERT EDGES / GRAPH DELETE EDGES (graph: bulk edge ingest has no batch form — every loader pays one statement per edge; add GRAPH INSERT/DELETE EDGES #369). The plan, staging, WAL, and Data Planes already carried EdgePutBatch / EdgeDeleteBatch; the statement layer above them was missing. The batch form takes a property-less VALUES list of (src, dst, label) triples, capped at 1000 edges per statement with the cap named in the error. The handler resolves each edge (surrogates, write policy) and buckets by home vShard, so a single-home batch is one apply burst per home; build_static_tx_class now derives participant homes and lock identity from batch edge plans for the Calvin path.

  2. feat(obsv): SHOW SNAPSHOT returns the monotonic WAL pin (wal_next_lsn) with node id and version, so a reader that pages with a fresh snapshot per page has a token to capture once and compare against a producer's commit marker (pgwire: no way to pin a read snapshot across pages — multi-page readers can observe half-applied producer batches; add pg_snapshot_xmin / SHOW SNAPSHOT #370). A saturated dispatch WFQ reported a generic dispatch error that the classifier leaves unclassified; it now reports a dispatch_capacity: detail classified as rate_exceeded, the same class vshard admission capacity uses, with a process-wide counter rendered as nodedb_dispatch_capacity_exhausted_total (cluster: a saturated dispatch queue has no documented retryable class — clients cannot back off correctly; add one and a counter #371). SystemMetrics gains graph_edges_written_total and graph_edges_deleted_total, incremented at the edge-write handler success points and rendered in /metrics and SHOW STATS (obsv: add loader-facing write-pressure counters (graph edge writes, dispatch capacity busy) #372).

  3. test(graph): wire coverage proving batch write paths honour RLS/GRANT (test(graph): batch write paths need RLS and GRANT probes #373). A batch delete under a FOR WRITE owner policy evaluates the policy per edge (conforming edge deleted, denied edge survives, statement reports an error); a property-less batch insert under an owner policy is refused with nothing landing, matching the existing batch-edge decision in the RLS injection pass.

  4. feat(obsv) (2nd): a ten-second sampler loop publishes per-database bridge queue depth from the dispatch WFQ, with a read-only Dispatcher::db_queue_depth accessor. The other per-database families stay unwritten on purpose and the sampler documents why (the session registry has no production registration caller; memory, storage, WAL commit latency, and maintenance CPU have no per-database source).

Notes

  • The batch statement is property-less by design: BatchEdge carries no property object, so per-edge PROPERTIES stays on the single-edge form. That is also why the RLS injection pass refuses batch writes when a policy applies, which the new tests pin.
  • obsv: add loader-facing write-pressure counters (graph edge writes, dispatch capacity busy) #372 stays partial on purpose: the pre-existing bridge_queue_depth gauge has no writer anywhere in the tree (six per-database setters have no callers). That is a separate finding and is not touched here.
  • No 66k edge benchmark attached yet; the wire tests plus unit coverage are the evidence in this PR.

Evidence

  • nodedb-sql: 863 tests pass (5 new parser tests).
  • error_classify: unit test for the retryable dispatch capacity class.
  • Wire (real server): graph_dsl_batch 3/3, graph_batch_rls 3/3, show_snapshot 1/1; regressions graph_dsl 44/44, graph_timeseries_rls_probe 7/7, engine_surface_graph 14/14, pgwire_show_dispatch 19/19.
  • cargo check -p nodedb, cargo fmt, clippy -p nodedb-sql --all-targets clean.

One machine-load flake was observed in this session on graph_path_preserves_colon_containing_user_id under a parallel run; it passes when run alone and in sequence, so it is noted rather than treated as a regression.

What CI does not cover locally

  • 32-bit and ASan fuzz targets, and the full nightly matrix.
  • No performance bench; run-ci requested for the full suite.

Closes #369
Closes #370
Closes #371
Closes #373
Refs #372
Refs #375

Bulk loaders pay one GRAPH INSERT EDGE per edge, so a 66k edge import
spends hours in round trips. The plan, staging, WAL, and Data Planes
already carried EdgePutBatch / EdgeDeleteBatch; the statement layer above
them was the missing piece.

Add GRAPH INSERT EDGES / GRAPH DELETE EDGES taking a property-less VALUES
list of (src, dst, label) triples, capped at 1000 edges per statement with
the cap named in the error. Per-edge PROPERTIES stays on the single-edge
form until BatchEdge can carry a property object.

The handler resolves each edge (surrogates, write policy) and buckets by
home vShard, so a single-home batch is one apply burst per home and the
Calvin path sequences the whole batch as one tx class. build_static_tx_class
now derives participant homes and lock identity from batch edge plans.
Copilot AI lite review requested due to automatic review settings September 24, 2026 12:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@EnRaiha EnRaiha added the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 24, 2026
Three loader-facing gaps on one surface.

SHOW SNAPSHOT returns the monotonic WAL pin (wal_next_lsn) with the node id
and version, giving a reader that pages with a fresh snapshot per page a
token to capture once and compare against a producer's commit marker.

A saturated dispatch WFQ reported a generic dispatch error, which the
classifier leaves unclassified, so a loader had no retry contract. It now
reports a `dispatch_capacity:` detail that classifies as rate_exceeded in
the same class vshard admission capacity uses, counted process-wide and
rendered as nodedb_dispatch_capacity_exhausted_total.

SystemMetrics gains graph_edges_written_total and
graph_edges_deleted_total, incremented at the edge-write handler success
points and rendered in /metrics and SHOW STATS, so pacing can read arrival
versus apply rate instead of estimating from query counts.
Coverage for the batched edge statements: a batch delete under a FOR WRITE
owner policy evaluates the policy per edge (the conforming edge is deleted,
the denied one survives, and the statement reports an error), and a
property-less batch insert under an owner policy is refused with nothing
landing. That refusal matches the existing batch-edge decision in the RLS
injection pass, pinning that a batch is never the write surface a policy
does not reach.
The per-database bridge_queue_depth gauge had no writer: nothing filled it
from the dispatch WFQ, so the exported line was a constant zero for every
database. Add a ten-second background loop that reads each database's WFQ
depth across all cores and publishes it, backed by a read-only Dispatcher
accessor for the sum.

The other per-database families stay unwritten on purpose, and the sampler
documents why: the session registry that would feed connections has no
production registration caller, and memory, storage, WAL commit latency,
and maintenance CPU have no per-database source at all.
@EnRaiha
EnRaiha force-pushed the feat/etl-batch-and-loader-observability branch from 4509e64 to a08af14 Compare September 24, 2026 14:36
The connections gauge had no writer. Fill it from the admission registry's
per-database live permit count, resolved by name the way SHOW DATABASE
USAGE resolves it, and publish connections before the dispatcher lock is
touched so a contended dispatch poller cannot stall the sample.

An entry exists once a quota record applies, and a database with no entry
stays unpublished: absence means unmeasured, never a fabricated zero.
Removing a cap keeps the entry and its live count, so a measured zero still
publishes.
@EnRaiha

EnRaiha commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by four focused pull requests, each rebased onto current main and carrying one issue.

Why this PR is superseded

The branch is 68 commits behind main, mergeable is CONFLICTING, and one part of it is now wrong rather than only stale.

main gained DispatchCapacity as a typed error with DispatchCapacityScope (TenantInflight, DatabaseSuspended, QueueFull) in a commit dated 2026-09-23, one day before this branch was written. It lands through #392 and splits bridge/dispatch/dispatcher.rs into enqueue.rs, refusal.rs, and response_poll.rs.

This PR reports a saturated dispatch queue by string-sniffing a dispatch_capacity: prefix on Error::Dispatch and classifying it as rate_exceeded. main now carries the same condition as a typed variant that classifies as server_overload (57P03). Merging the string form would put two different retryable classes on one condition.

Where each piece went

Issue Replacement
#369 batch edge writes pr/374-369-batch-edge-writes
#370 SHOW SNAPSHOT pin pr/374-370-show-snapshot
#371 dispatch capacity signal pr/374-371-dispatch-capacity-counter
#372 graph edge write counters pr/374-372-edge-write-counters
#373 RLS and GRANT probes shipped inside pr/374-369-batch-edge-writes

#373 was already closed while its work sat only in this branch, so the merge could not close it.

Evidence carried over

Each replacement PR was built from current main, with its own red proof (the code under test removed, the new test failing, then restored) and a green run on the rebased head. Two findings from this branch are recorded rather than carried:

  • GRAPH DELETE EDGES loops the single-edge path, so it wins at the parser and not at the executor. EdgeDeleteBatch exists in the data plane and no handler calls it.
  • A cross-shard edge is written on its src home in single-node deployments, so the destination home gets no dispatch of its own.

Thanks for the review time on this one.

@EnRaiha EnRaiha closed this Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Opt this PR into the full test suite; re-add to force a re-run

Projects

None yet

2 participants