fix(derive): selectivity-aware per-rule estimate for shared-variable equality joins (E-PLAN-003 q-error)#472
Closed
Disentinel wants to merge 2 commits into
Closed
fix(derive): selectivity-aware per-rule estimate for shared-variable equality joins (E-PLAN-003 q-error)#472Disentinel wants to merge 2 commits into
Disentinel wants to merge 2 commits into
Conversation
Self-analyze of the grafema monorepo (~714k nodes / 1.33M edges) failed three different ways; all three were conservative limits / a concurrency bug that only bite at self-scale. With these, the full derive phase completes (40 packs, 0 SIGSEGV, 0 plan rejects). 1. cap-lift (E-EXEC-001 sibling): the @materialize batch path used the interactive EvalLimits::default() 100k max_intermediate_results. The deadline was already lifted (RFDB_MATERIALIZE_DEADLINE_SECS) but the intermediate cap was not; batch derive of @stdlib/js_local_refs overflows it. Lift it for the batch path (default unbounded, env RFDB_MATERIALIZE_MAX_INTERMEDIATE). 2. compaction vs parallel-derive heap corruption (the hard one): the materialize handler holds a db.engine.read() lock but commits derived edges across 40 packs; those commits auto-triggered compaction (rayon, memmap2) which rewrites/unmaps segments WHILE the same materialize's par_join_rows rayon tasks read them -> use-after-unmap heap corruption -> SIGSEGV (confirmed: TSan named join_derived/par_join_rows; rayon=1 passes; disabling compaction passes). Fix: AutoCompactionSuppressGuard suppresses auto-compaction for the materialize phase; deferred compaction runs at the natural barrier (end_bulk_load / explicit Compact) under the exclusive write lock. Correct phase serialization, not masking. 3. MAX_MATERIALIZED_FACTS planner guard (E-PLAN-003): static_member in js_property_access_full estimates 11.7M facts > the 10M guard and is rejected — but the ACTUAL output is ~811 edges (a ~14,000x cardinality q-error: the estimator cross-products relation sizes, ignoring the highly-selective file+class+method join keys). Make the guard env-overridable (RFDB_MAX_MATERIALIZED_FACTS, default 10M). Deeper fix = selectivity-aware estimation (follow-up). Self-analyze env for a full run: RFDB_MATERIALIZE_DEADLINE_SECS=3600 RFDB_MAX_MATERIALIZED_FACTS=200000000 Still open (separate): RPC 60s client timeouts + slow clear/drop (wall 3); the estimator q-error; full exit-0 + mcp:tool=27 verification in flight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…equality joins The planner's per-rule output estimate (E-PLAN-003 guard) cross-producted every generator's RAW relation magnitude along the join, ignoring that a later leg applying an EQUALITY selection on a generator-introduced key against an already-bound value makes that join nearly 1:1. The stdlib `static_member` rule (js_property_access_full) — uc_pa · node(Cls,"CLASS") · attr(Cls,"file",F) · attr(Cls,"name",BaseN) · edge(Cls,T,"HAS_METHOD") · attr(T,"name",N) — was sized at ~11.7M (|uc_pa|·|CLASS|·avg_degree) and spuriously rejected, vs an ACTUAL 811 materialized edges (~14000x over-estimate). The branch worked around it with the RFDB_MAX_MATERIALIZED_FACTS env override; this is the principled fix. When a generator-introduced variable is EQUALITY-PINNED by a downstream base-relation selection whose other positions are all already-bound (a shared-variable join key — attr(v,key,BoundVal) / attr(BoundId,key,v) / edge(v,BoundEndpoint,_)), the generator is in truth a keyed probe: reduce its multiplicative contribution by the existing key-narrowing (√) factor instead of multiplying the full relation magnitude. A key with NO downstream equality is unreduced, so a GENUINE cross-product (whose legs share no pinning equality) keeps its full magnitude and still trips the guard — the guard's job is intact. Sizing/ordering can only let more rules pass, never change which facts derive (I1). Tests: static_member-shape selective join now plans under the 10M default guard; a genuine two-hop explosion still trips E-PLAN-003; direct discrimination unit on the pin detector (pins a bound-value equality, not a type-restriction / free- endpoint hop / free-value read). All 13 plan tests + 323 derive tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Superseded by the consolidated PR #475 (feat/self-analyze-hardening) — all 6 branches merged clean into one. Commits preserved there. |
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.
Problem
The derive planner's per-rule output estimate (the
E-PLAN-003/MAX_MATERIALIZED_FACTSguard) cross-products every generator's raw relation magnitude along the join, ignoring that a later leg applying an equality selection on a generator-introduced key against an already-bound value makes that join nearly 1:1.The stdlib
static_memberrule (js_property_access_full.dl):estimated ~11.7M facts on the full monorepo self-analyze graph (|uc_pa| × |CLASS| × avg-degree) and was rejected with
E-PLAN-003, vs an actual ~811 materialized edges — a ~14,000× over-estimate. Thefile/class-name/method-nameequality joins are nearly 1:1, but the estimator never accounted for them. The base branch (fix/derive-batch-cap, d26a1e5) worked around it with theRFDB_MAX_MATERIALIZED_FACTSenv override and explicitly flagged "deeper fix = selectivity-aware estimation (follow-up)." This PR is that follow-up.Fix (principled, not "raise the constant")
When a generator-introduced variable is equality-pinned by a downstream base-relation selection whose other positions are all already-bound — a shared-variable join key:
attr(v,key,BoundVal)/attr(BoundId,key,v)/edge(v,BoundEndpoint,_)— the generator is in truth a keyed probe, not a full scan. Its multiplicative contribution is reduced by the existing key-narrowing (√) factor per pinned key instead of multiplying the full relation magnitude.A key with no downstream equality is left unreduced, so a genuine cross-product (whose legs share no pinning equality) keeps its full magnitude and still trips the guard — the guard's job is intact. The reduction is a pure ordering/sizing change: it can only let more rules pass, never change which facts a rule derives (I1).
The pin walk advances the running bind set across intervening legs, so an equality whose value side is bound by a sibling leg between the generator and the filter is still recognized.
Verification
Unit (plan.rs):
selective_equality_join_estimate_is_realistic_static_member_shape— the static_member-shaped selective join now plans under the 10M default guard.genuine_cross_join_without_selective_key_still_trips_guard— a real two-hop explosion (edge(X,Y,"T"), edge(Y,Z,"T")on a dense graph) still tripsE-PLAN-003.downstream_equality_pin_detection_discriminates— the pin detector fires on a bound-value equality, NOT on a bare type-restriction, a free-endpoint hop, or a free-value read.End-to-end (real graph, grafema-dev): ran a real-graph planner probe over the shipped
js_property_access_fullpack against the live 503k-node / 1.1M-edge self-analyze store, withoutRFDB_MAX_MATERIALIZED_FACTS— the pack plans (maxstatic_memberestimate 7.17M ≤ 10M default guard). The env override is no longer required for this pack on that graph.🤖 Generated with Claude Code