Skip to content

JNeo4jBackend: _VALUE_REACHES and _SLICE bind the path, so an unbounded SDG query enumerates trails instead of pruning #402

Description

@rahlk

Describe the bug

JNeo4jBackend._VALUE_REACHES (cldk/analysis/java/neo4j/neo4j_backend.py:984) and JNeo4jBackend._SLICE (:898) both bind the whole path so they can apply the in-application predicate to every node on it:

MATCH (a:JBodyNode {id:$src}) WHERE a.id STARTS WITH $prefix
MATCH p = (a)-[:J_DDG|J_CDG|J_PARAM_IN|J_PARAM_OUT|J_SUMMARY*1..]->(m:JBodyNode)
  WHERE m.id STARTS WITH $prefix AND all(n IN nodes(p) WHERE n.id STARTS WITH $prefix)
WITH DISTINCT m WHERE m.id IN $dsts RETURN count(m) > 0 AS ok

Binding p makes Cypher enumerate trails, not visit nodes. WITH DISTINCT m dedupes the result after the expansion, not during it — so the comment at :977 ("WITH DISTINCT m before the membership test is what makes this a pruning BFS instead of a trail enumeration") does not hold once the relationship subgraph has cycles. The ThingsBoard measurements quoted there were taken at depth 5, where the difference is invisible; the SDG relationship set is cyclic, and the cost is superlinear in depth.

Measured on a DayTrader UKG (5 microservices, 1,736 callables, one application namespace), cldk==2.0.0rc6, Neo4j 5.x, single instance, no other load. Queries run verbatim from the source with a client-side transaction timeout.

_VALUE_REACHES, seed = TradeAppServlet.performTask's first parameter, targets = the formal-in vertices of TradeServletAction.doLogin; forward SDG closure of that seed is 1,621 distinct nodes:

depth time result
3 0.21 s False
5 0.52 s True
8 3.65 s True
12 52.67 s True
unbounded (depth=None) still running at 30 min, killed

_SLICE, forward, seed = the same callable's second parameter, whose closure is 17 distinct nodes:

depth _SLICE frontier walk nodes returned
3 0.22 s 0.43 s 17
5 0.58 s 0.45 s 17
8 3.73 s 0.44 s 17
unbounded TransactionTimedOut at 60 s 0.44 s 17

Seventeen nodes is the whole answer, and unbounded still cannot produce it — the blowup is trail count, not graph size.

The practical effect is that the documented way to ask these questions does not return. JavaAnalysisBackend.flows_to_call and flows_to_argument pass depth straight through, and depth=None is their default, so any caller that does not guess a bound hangs. A consumer of ours ran 32 concurrent unbounded flows_to_call calls and none returned in 30 minutes; SHOW TRANSACTIONS showed 33 running with the oldest at PT30M34S. Guessing a bound is not a fix: at depth 3 the seed above answers False and at depth 5 it answers True, so a bound low enough to return is low enough to be wrong.

To Reproduce

Not stated in the original issue.

Expected behavior

  • Both queries return in seconds with depth=None on a graph where the current implementation does not return within 30 minutes. Report the number, not "fast".
  • Answers match the current implementation exactly at every depth the current one can finish: on the seeds above, _VALUE_REACHES gives False, True, True, True at depths 3, 5, 8, 12 and _SLICE gives the same 17-element node set at depths 3, 5, 8.
  • A test that fails before and passes after, with a cyclic seed, asserting flows_to_call(..., depth=None) returns the expected boolean under a wall-clock bound.
  • flows_to_call / flows_to_argument / flow_slice unchanged at the public surface — existing callers need no edit.

Additional context

Scope boundary

Rewrites _VALUE_REACHES and _SLICE only, and only their traversal strategy — same signatures, same return types, same in-application semantics, same depth meaning (hop count from the seed).

Not _PATHS (:930): it uses allShortestPaths, which the planner bounds, and it genuinely needs nodes(p) because it returns the paths. Not the Python or TypeScript backends. Not the networkx backend, whose call_reaches is a different code path. No change to the relationship set, the can:// id grammar, or the analyzer floor.

Goals

  • Replace _VALUE_REACHES with a hop-at-a-time frontier expansion: one indexed query per hop over the prefix-induced subgraph, each node visited once, stop at the first hop containing a target.
  • Replace _SLICE's traversal the same way, accumulating the visited set and applying the existing ORDER BY/cap paging to it.
  • Keep the whole-path in-application semantics: a node is reachable only via nodes that are themselves in the application. (This is what makes the rewrite sound — the predicate is a restriction to the induced subgraph, so reachability there is exactly a walk within it.)
  • Correct or remove the :977 comment and its ThingsBoard numbers; if the numbers stay, note the depth they were taken at and that they do not extend to unbounded.
  • Regression test at a depth the current query cannot finish, so the fix cannot silently revert.

Caveats and known risks

  • Round trips. A frontier walk is one query per hop instead of one query total, so a shallow call gets slower: on the _SLICE seed above, 0.43 s against 0.22 s at depth 3. That is the trade — bounded worst case in exchange for a worse best case. If the regression matters, keep the single-query form for small depth and switch above a threshold; measure before adding the branch.
  • depth semantics must not drift. The current query counts relationships in the path; the frontier form counts hops from the seed. They agree, but only if the walk increments once per expansion and *0.. in _SLICE keeps including the seed itself.
  • Equal answers, not equal paths. Trails and walks agree on reachability — if a walk exists a simple path exists, and BFS reaches at the minimum hop count, so a depth bound cuts the same set. Anything that wanted trail multiplicity would change behaviour, but neither query exposes it (count(m) > 0, DISTINCT m.id).
  • Not fixed by a server-side timeout. db.transaction.timeout bounds the damage but turns a hang into an error, and the caller cannot distinguish "no flow" from "gave up" — it must not become a silent False.
  • Cyclic SDG is the trigger, so a small graph is not proof of a fix. A regression test whose closure is acyclic will pass against the current query too; the seed has to sit in a cycle.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions