Repository navigation
[dbsp] Count the runs of a nested circuit that get no input - #7743
Open
anandbraman wants to merge 2 commits into
Open
anandbraman wants to merge 2 commits into
anandbraman wants to merge 2 commits into
Conversation
A nested circuit runs to a fixed point once in every step of its parent, also when none of its imports has data. In transaction mode one commit takes several root steps, so a recursive view can run many times per commit with nothing to do. The circuit profile shows the steps and the runtime of the nested circuit, but not how many of its runs had input. Add three counters to each nested circuit node, reported in the circuit profile: - subcircuit_runs_count: runs, one per parent step. - subcircuit_runs_with_input_count: runs in which at least one import was not empty on this worker. - subcircuit_runs_input_unknown_count: runs in which an import could not tell if its value was empty. An import operator now reports if its last imported value was empty (`ImportOperator::imported_empty`). `Delta0` implements it; other import operators report that they cannot tell. Add a test that measures runs, runs with input, and root and inner steps per commit for a transitive closure next to a chain of joins, with 1 and 4 workers. Signed-off-by: Anand Raman <anand.raman@feldera.com>
Contributor
|
What I ran:
The table I got matches the PR description: runs = root steps, and 0 runs with input for "facts only" and "empty".
Otherwise LGTM once the profiler aggregation is fixed (see the inline comment). |
Review fixes for the nested circuit run counters: - `ImportAccumulator`, the import of `non_incremental` circuits, now reports if its imported batch was empty. Before, every run of such a circuit counted as a run with unknown input. - profiler-lib takes the largest reading of the three run counts, like `steps_count`, instead of adding them up over the workers. - The description of `subcircuit_runs_count` says that a non-incremental circuit only collects its input until it is complete. - New tests: an import that cannot tell if its input was empty, and a non-incremental circuit. Signed-off-by: Anand Raman <anand.raman@feldera.com>
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.
Linked Issue
No issue yet. This adds the measurements for two planned issues: skip a run of a recursive sub-circuit that has no input, and let a sharded accumulator receiver finish its flush in the step in which its peers flush.
What this changes
A nested circuit runs to a fixed point once in every step of its parent, also when none of its imports has data. In transaction mode one commit takes several root steps, so a recursive view can run many times per commit with nothing to do. The circuit profile shows the steps and the runtime of a nested circuit, but not how many of its runs had input.
This PR adds three counters to each nested circuit node. They appear in the circuit profile, and so in support bundles:
subcircuit_runs_countsubcircuit_runs_with_input_countsubcircuit_runs_input_unknown_countHow:
ImportOperator::imported_empty()reports if the last imported value was empty. The default isNone.Delta0recordsval.is_zero(), andImportAccumulator(the import ofnon_incrementalcircuits) recordsval.is_empty(), on each import.Delta0NonIterativekeeps the default: it has no callers, and itsD: Clonebound has no emptiness test.ChildNode::evalclassifies each run after it runs the import nodes, andChildNode::metadatareports the counts.AggregationMode.Max), likesteps_count, instead of adding them up over the workers.The counters do not change what the circuit computes or when it runs.
A
non_incrementalcircuit is also run once per parent step, but it only collects its input until the step in which the input is complete. The description ofsubcircuit_runs_countsays so.Describe Manual Test Plan
New tests in
operator::dynamic::recursive::test::run_stats. Each builds a nested circuit overedgesand, next to it, a chain of 8 joins overfacts, so that a commit takes several root steps. Each measures root steps, inner steps (from scheduler events) and the new counters for each commit, with 1 and 4 workers. Run them with--nocaptureto see the tables.recursion_runs_at_every_root_stepdelta0edgeschanged; no unknown runsopaque_import_counts_runs_as_unknownimported_emptynon_incremental_runs_at_every_root_stepnon_incrementalcircuitedgeschanged; no unknown runsResults with 4 workers (debug build):
With 1 worker, every commit takes 1 root step and 1 run.
The test asserts today's behavior: the nested circuit runs once per root step. A change that skips runs without input must update the test.
Checks:
Delta0always reports "empty"; the run count counts only runs with input;ImportAccumulatorkeeps the default). The tests failed each time.cargo test -p dbsp --lib -- recursive nonlinear_recursion profile delta0 non_incremental: 67 passed.profile.test.tspasses (179 tests) under the Bun runtime. With the oldprofile.ts, the newmode(...)assertions fail. Under Node 20 the file does not load (entries().mapneeds iterator helpers);regionSize.test.tsandviewport.test.tsfail the same way with and without this change.cargo clippy -p dbsp --lib --tests -- -D warningsandcargo fmt: clean. The pre-commit hooks passed.Checklist
Breaking Changes?
Mark if you think the answer is yes for any of these components:
Describe Incompatible Changes
None. The profile metric catalog gets three new entries.