Conversation
✅ Deploy Preview for meta-velox canceled.
|
Contributor
|
@Yuhta has exported this pull request. If you are a Meta employee, you can view the originating Diff in D118870175. |
meta-cla
Bot
added
the
CLA Signed
Selective Build Plan
Affected targets (428)Directly changed (92)
Transitively affected (336)
Fast path • Graph from main@0a0871f821f0 |
CI Failure Analysis
❌ Build with GCC / Linux release with adapters — BUILD Failure View logsBuild errors: 8 compilation errors in Each error is on a Correlation with PR changes: This is directly caused by the PR. The PR renamed Known issues:
Recommended fix: Add ConvergenceConfig{
.maxIterations = 0, .errorWhenMaxIterationReached = false},to: ConvergenceConfig{
.plans = {},
.maxIterations = 0,
.errorWhenMaxIterationReached = false},at lines 620, 658, 671, 683, 730, 753, 764, and 786. |
Yuhta
added a commit
to Yuhta/axiom
that referenced
this pull request
Sep 11, 2026
Summary: X-link: facebookincubator/velox#18854 PageRank's stopping rule is the RMSE of the per-vertex rank change, measured *after* the update. That statistic does not exist until the body's last plan has produced the new ranks -- and that plan's output *is* the state the iteration commits, so the reduce that turns per-worker partials into one verdict has nowhere to go inside the body. Reducing it needs a shuffle of its own: each worker reduces its shard, the partials exchange, one final aggregation emits the verdict. A Velox plan fragment ends at a shuffle boundary, so that is not one plan but a chain of them -- which a single `ConvergenceConfig::plan` cannot express. `ConvergenceConfig::plan` becomes `plans`, chained exactly as `FixedPointNode::plans()` are: the first starts with a `StateSourceNode`, every later one starts with an `Exchange`, every non-last one ends with a `PartitionedOutput`, and the last emits the single BOOLEAN verdict. `converging(plan, n)` still takes one plan and delegates to the vector overload, so existing callers are unchanged. `FixedPointNode` validates that the chain really is that: exactly one incoming shuffle per fragment, so a branching topology such as a distributed join is rejected rather than silently half-checked on the primary input chain only; matching schemas across each shuffle boundary; one partition count across every stage, since taking the maximum does not make incompatible counts safe; and the last plan's single BOOLEAN column. `numWorkers()` and `requiresSplits()` account for the convergence chain because a convergence sequence that reduces across workers needs coordinator-assigned peers just as a shuffling body does: consult the body alone and a non-shuffling body with a shuffling convergence chain reports one worker and no split requirement, leaving its exchanges waiting on peers nobody assigned. Serialization round-trips the chain. Also adds `ConvergenceConfig::whenDeltaEmpty(maxIterations)`, the semi-naive termination test -- stop on the first iteration that writes no rows -- which needs no convergence plan at all, and so no sub-task per iteration to recompute a row count the framework already holds. Restricted to a non-shuffling fixed point, because the delta is a worker's local shard. Worker propagation through a *nested* fixed point is split into the next diff. Plan-node half only; the execution that consumes it is at the top of the stack. Differential Revision: D118870175
Summary: X-link: facebookincubator/axiom#1834 PageRank's stopping rule is the RMSE of the per-vertex rank change, measured *after* the update. That statistic does not exist until the body's last plan has produced the new ranks -- and that plan's output *is* the state the iteration commits, so the reduce that turns per-worker partials into one verdict has nowhere to go inside the body. Reducing it needs a shuffle of its own: each worker reduces its shard, the partials exchange, one final aggregation emits the verdict. A Velox plan fragment ends at a shuffle boundary, so that is not one plan but a chain of them -- which a single `ConvergenceConfig::plan` cannot express. `ConvergenceConfig::plan` becomes `plans`, chained exactly as `FixedPointNode::plans()` are: the first starts with a `StateSourceNode`, every later one starts with an `Exchange`, every non-last one ends with a `PartitionedOutput`, and the last emits the single BOOLEAN verdict. `converging(plan, n)` still takes one plan and delegates to the vector overload, so existing callers are unchanged. `FixedPointNode` validates that the chain really is that: exactly one incoming shuffle per fragment, so a branching topology such as a distributed join is rejected rather than silently half-checked on the primary input chain only; matching schemas across each shuffle boundary; one partition count across every stage, since taking the maximum does not make incompatible counts safe; and the last plan's single BOOLEAN column. `numWorkers()` and `requiresSplits()` account for the convergence chain because a convergence sequence that reduces across workers needs coordinator-assigned peers just as a shuffling body does: consult the body alone and a non-shuffling body with a shuffling convergence chain reports one worker and no split requirement, leaving its exchanges waiting on peers nobody assigned. Serialization round-trips the chain. Also adds `ConvergenceConfig::whenDeltaEmpty(maxIterations)`, the semi-naive termination test -- stop on the first iteration that writes no rows -- which needs no convergence plan at all, and so no sub-task per iteration to recompute a row count the framework already holds. Restricted to a non-shuffling fixed point, because the delta is a worker's local shard. Worker propagation through a *nested* fixed point is split into the next diff. Plan-node half only; the execution that consumes it is at the top of the stack. Differential Revision: D118870175
meta-codesync
Bot
changed the title
Yuhta
force-pushed
the
export-D118870175
branch
from
September 11, 2026 14:47
f07d534 to
18918d9
Compare
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
1 participant
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.
Summary:
X-link: facebookincubator/axiom#1834
PageRank's stopping rule is the RMSE of the per-vertex rank change, measured
after the update. That statistic does not exist until the body's last plan
has produced the new ranks -- and that plan's output is the state the
iteration commits, so the reduce that turns per-worker partials into one
verdict has nowhere to go inside the body.
Reducing it needs a shuffle of its own: each worker reduces its shard, the
partials exchange, one final aggregation emits the verdict. A Velox plan
fragment ends at a shuffle boundary, so that is not one plan but a chain of
them -- which a single
ConvergenceConfig::plancannot express.ConvergenceConfig::planbecomesplans, chained exactly asFixedPointNode::plans()are: the first starts with aStateSourceNode, everylater one starts with an
Exchange, every non-last one ends with aPartitionedOutput, and the last emits the single BOOLEAN verdict.converging(plan, n)still takes one plan and delegates to the vectoroverload, so existing callers are unchanged.
FixedPointNodevalidates that the chain really is that: exactly one incomingshuffle per fragment, so a branching topology such as a distributed join is
rejected rather than silently half-checked on the primary input chain only;
matching schemas across each shuffle boundary; one partition count across every
stage, since taking the maximum does not make incompatible counts safe; and the
last plan's single BOOLEAN column.
numWorkers()andrequiresSplits()account for the convergence chain because a convergence sequence that
reduces across workers needs coordinator-assigned peers just as a shuffling
body does: consult the body alone and a non-shuffling body with a shuffling
convergence chain reports one worker and no split requirement, leaving its
exchanges waiting on peers nobody assigned. Serialization round-trips the
chain.
Also adds
ConvergenceConfig::whenDeltaEmpty(maxIterations), the semi-naivetermination test -- stop on the first iteration that writes no rows -- which
needs no convergence plan at all, and so no sub-task per iteration to recompute
a row count the framework already holds. Restricted to a non-shuffling fixed
point, because the delta is a worker's local shard.
Worker propagation through a nested fixed point is split into the next diff.
Plan-node half only; the execution that consumes it is at the top of the stack.
Differential Revision: D118870175