Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport bundle activation performance and log mask race condition fixes #1595

Merged
merged 6 commits into from
Jul 27, 2019

Conversation

tsandall
Copy link
Member

This PR backports for the fixes in #1516 and #1551 to the 0.12 release.

tsandall and others added 6 commits July 27, 2019 13:27
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
These changes update the storage to accept arbitrary key/value
parameters when creating transactions. The container is passed to
trigger callbacks that are invoked when transactions commit.

The use case for this is avoiding parse and compile operations on
modules loaded out of bundles. The parse and compile step on large
bundles can take several seconds. Since the triggers are executed
while the store's read-lock is held, policies queries block.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
These changes update the manager and bundle plugin to avoid parsing
and compiling modules during the manager's trigger callback. Since
policy queries are blocked while triggers execute, it's adavantageous
to cache the compiler that is obtained during bundle activation and
reduce the work done in the trigger callback.

Also, as part of these changes, the bundle plugin incorporates
remaining modules when it recompiles. This ensures that remaining
modules are correct.

Fixes open-policy-agent#1515

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
We were only using it on the prepare step, but we needed to use the
passed in txn each time we eval the prepared query.

Signed-off-by: Patrick East <east.patrick@gmail.com>
When doing a prepared query Eval() or Partial() we would build an
EvalContext that defaulted to using the original Rego objects
transaction. This is problematic as that one might have been aborted,
or just be old. The expectation if you don't specify one at Eval()
time was that you would get a new one at the current state of the
store, so we should keep the prepared Eval() to work the same way.

This means that if you did have a specific transaction you wanted
to use you would now need to pass it in instead of only setting
it on the original Rego object. This changed some tests but in real
usage it shouldn't affect much.

This also fixes an issue where we would abort transactions auto
created on the EvalContext way too early. The helper (internal to
the Rego package) now returns a finish function that will do the
right thing and abort it at an appropriate time.

Signed-off-by: Patrick East <east.patrick@gmail.com>
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
@tsandall tsandall merged commit c9fd156 into open-policy-agent:release-0.12 Jul 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants