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

broker: prevent overlap of policy evaluations #812

Merged
merged 4 commits into from
Jan 2, 2024

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Dec 22, 2023

When a policy is sent for evaluation by a handler, it is placed into the eval broker queue. If the queue already has an eval for the policy, it discards one of them, keeping the most recent one. This is used to guarantee that a policy is never processed in parallel by different workers.

The code had the return statement in the wrong scope, causing a duplicate eval to be enqueued even when the policy was already been processed, resulting in unexpected concurrent policy evaluations.

Closes #610

When a policy is sent for evaluation by a handler, it is placed into the
eval broker queue. If the queue already has an eval for the policy, it
discards one of them, keeping the most recent one. This is used to
guarantee that a policy is never processed in parallel by different
workers.

The code had the `return` statement in the wrong scope, causing a
duplicate eval to be enqueued even when the policy was already been
processed, resulting in unexpected concurrent policy evaluations.
Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Is it possible to add a test that covers this behaviour?

CHANGELOG.md Outdated Show resolved Hide resolved
lgfa29 and others added 2 commits January 2, 2024 13:32
Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
@lgfa29
Copy link
Contributor Author

lgfa29 commented Jan 2, 2024

LGTM.

Is it possible to add a test that covers this behaviour?

Ah yeah, good point. Expanded TestBroker in ff181cd to cover this.

@lgfa29 lgfa29 merged commit 5888bec into main Jan 2, 2024
24 checks passed
@lgfa29 lgfa29 deleted the b-fix-policy-eval-overlap branch January 2, 2024 18:41
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.

double scaling action
2 participants