Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(sysflow): fix double call to StartWorkers() function when policies are reloaded #59

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

msardara
Copy link
Contributor

@msardara msardara commented Feb 12, 2024

When a new policy file is loaded, sysflow rebuilds the policies using the function createPolicyInterpreter, that creates the policies and at the end calls the pi.StartWorkers(), that is in charge to start the worker processing the records and applying the rules.

The issue comes from the fact that the very same function createPolicyInterpreter is also used when a new modification to the policy files is detected in CheckForPolicyUpdate:

  • createPolicyInterpreter creates new policies and calls pi.StartWorkers()
  • CheckForPolicyUpdate sends a notification to p.interChan <- pi
  • the method Process in policyengine sees this notification and calls s.pi.StopWorkers() and s.pi.StartWorkers()

StartWorkers gets called twice, while StopWorkers is called once. This leads to concurrency issue, in particular pi.workerCh and pi.wg gets replaced twice in StartWorkers leading to inconsistencies in the wg counter.

…are reloaded (#1)

Ref: EXP-2010

Signed-off-by: Mauro Sardara <msardara@cisco.com>
@msardara msardara changed the title fix(sysflow): fix double call to StartWorkers() function when policy are reloaded fix(sysflow): fix double call to StartWorkers() function when policies are reloaded Feb 12, 2024
@araujof araujof self-requested a review February 13, 2024 15:08
@araujof araujof added the bug Something isn't working label Feb 13, 2024
Copy link
Member

@araujof araujof left a comment

Choose a reason for hiding this comment

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

LGTM!
Nice catch, @msardara! Thanks for this fix.

@araujof araujof merged commit bd025c6 into sysflow-telemetry:dev Feb 13, 2024
13 checks passed
araujof pushed a commit that referenced this pull request Jan 15, 2025
… reloaded (#59)


Signed-off-by: Mauro Sardara <msardara@cisco.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants