compile,planner: improve determinism of plan/wasm bundle builds#8732
Open
philipaconrad wants to merge 1 commit into
Open
compile,planner: improve determinism of plan/wasm bundle builds#8732philipaconrad wants to merge 1 commit into
plan/wasm bundle builds#8732philipaconrad wants to merge 1 commit into
Conversation
philipaconrad
requested review from
anderseknert,
johanfylling,
srenatus and
sspaink
philipaconrad
force-pushed
the
philip/planner-determinism-fixes
branch
from
030a404 to
9a96da6
Compare
philipaconrad
commented
Jun 3, 2026
|
|
||
| // We sort rules by ref length, to ensure that when merged, we can detect conflicts when one | ||
| // rule attempts to override values (deep and shallow) defined by another rule. | ||
| sort.Slice(rules, func(i, j int) bool { |
Member
Author
There was a problem hiding this comment.
Per the golang docs, it's possible to get several different, valid sorting orders, depending on how the inputs arrive.
philipaconrad
force-pushed
the
philip/planner-determinism-fixes
branch
2 times, most recently
from
088abae to
d0f7a02
Compare
This commit fixes an issue where `plan` and `wasm` bundle build targets could produce different output bytes across separate invocations of `opa build` for the same inputs. There were two underlying causes, both from Golang random map iteration order leaking through to the order-sensitive planner. Causes: - compilePlan (v1/compile) and planQuery (v1/rego) iterated over the compiler's module map without sorting keys first. This caused the planner to have iteration-dependent variations in output. This was fixed by sorting the module names before use. - planRules (internal/planner) sorted rules by length of the rule name ref, which is not a unique value. Because the sorting of the rules was using an unstable sorting algorithm, and the rule names were coming from iterating over a `map` type in the rule trie, this had edge cases where non-deterministic output ordering could creep in. This was fixed by adding a ref `Compare` call as a tie-breaker to get a stable sorting order, regardless of iteration order in the rule trie. This commit also adds regression tests that assert plan output is independent of module and rule ordering. The two fixes are needed together because both sets of issues hit the planner from different angles, and are mostly independent of each other. Signed-off-by: Philip Conrad <philip_conrad@apple.com>
philipaconrad
force-pushed
the
philip/planner-determinism-fixes
branch
from
d0f7a02 to
1a4dcb5
Compare
Member
Author
|
ℹ️ Made a small refactor to use |
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.
What code changed, and why?
This PR fixes an issue where
planandwasmbundle build targets could produce different output bytes across separateopa buildinvocations for the exact same inputs. There were two underlying causes, both from Golang random map iteration order leaking through to the order-sensitive planner.Causes:
compilePlan(v1/compile) andplanQuery(v1/rego) iterated over the compiler's module map without sorting keys first. This caused the planner to have iteration-dependent variations in its output. This was fixed by sorting the module names before use.planRules(internal/planner) sorted rules by length of the rule name ref, which is not a unique value. Because the sorting of the rules was using an unstable sorting algorithm (the default in Golang), and the rule names were coming from iterating over amaptype in the rule trie, this had edge cases where non-deterministic output ordering could creep in. This was fixed by adding a refComparecall as a tie-breaker to get a stable sorting order over rule names, regardless of iteration order in the rule trie.This commit also adds regression tests that assert plan output is independent of module and rule ordering. The two fixes are needed together because both sets of issues hit the planner from different angles, and are mostly independent of each other.
How to test?
internal/plannerandv1/compile.