YTDB-634: Lazy ExecutionStream #869
YTDB-634: Lazy ExecutionStream #869sandrawar wants to merge 3 commits intoskip-path-constructionfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the recursive graph traversal mechanism in Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new LazyRecursiveTraversalStream class to replace the eager ArrayList materialization in recursive graph traversals, particularly for WHILE and maxDepth conditions. This new stream uses an explicit stack to avoid stack depth issues in deep traversals and enables lazy evaluation, which is beneficial for LIMIT clauses. The MatchEdgeTraverser was refactored to utilize this new lazy stream. New test cases were added to validate the lazy traversal's behavior with deep chains, limits, branching graphs, and max depth. A high-severity issue was identified in LazyRecursiveTraversalStream.pushFrame where the CommandContext could be left in an inconsistent state if an exception occurs during frame processing; a try-catch block is suggested to restore the context.
.../com/jetbrains/youtrackdb/internal/core/sql/executor/match/LazyRecursiveTraversalStream.java
Outdated
Show resolved
Hide resolved
Coverage Gate ResultsThresholds: 85% line, 70% branch Line Coverage: ✅ 94.3% (100/106 lines)
Branch Coverage: ✅ 84.3% (59/70 branches)
|
Test Count Gate Results✅ No baseline available yet — gate skipped (first run). |
edac01d to
5453cb1
Compare
d4507f8 to
0d6c1f8
Compare
5453cb1 to
6a61953
Compare
0d6c1f8 to
ca6ccc6
Compare
JMH LDBC Benchmark ComparisonBase: Single-Thread Results
Multi-Thread Results
Scalability (MT/ST ratio)
|
|
Hi @sandrawar, please profile regressions using asyncprofiler on Hetzner CCX 33 node and find out what caused the regressions. Please bear in mind that IC4 is quite noisy, so we need to likely find a better approach to run it, but it also can be that there is an issue in implementation, so we need to profile results for sure. |
6a61953 to
f1f81cc
Compare
ca6ccc6 to
083ce48
Compare
PR Title:
YTDB-634: Lazy ExecutionStream
Motivation:
MatchEdgeTraverser.executeTraversal() eagerly materializes all recursive results into an ArrayList before returning any of them:
This means the entire subtree is fully traversed and collected before the first result is yielded to the consumer. There is no backpressure — once recursion starts, it runs to completion. LIMIT cannot short-circuit the traversal, and peak memory is O(total_results).
This PR replaces the eager collection with LazyRecursiveTraversalStream — a stack-based DFS iterator that yields results one at a time. An explicit Deque replaces Java call-stack recursion: each frame tracks a node's self-result and its neighbor iteration state. The advance() method processes the stack iteratively — yield self, push neighbor frames, pop when exhausted — with O(1) call-stack depth regardless of traversal depth.
When the downstream consumer stops pulling (e.g., LimitExecutionStep is satisfied), unexplored subtrees are never expanded. For deep WHILE chains, the flat loop avoids StackOverflow risk that the previous recursive implementation had.