Fix actions not muting #506
Conversation
| if (rcaScheduler != null) { | ||
| rcaScheduler.updateAppContextWithMutedActions(actionsForMute); | ||
| } |
There was a problem hiding this comment.
This fixes this issue but we can run into such inconsistencies where changes may not reflect until a scheduler restart. Can the scheduler use a reference of the AppContext of RcaController instead of a copy ?
There was a problem hiding this comment.
That was my first thought as well. But I saw this comment on L215 and decided to go this way.
// RcaScheduler should be started with a snapshot of the AppContext as RcaController
// monitors it for stale state and always restarts the scheduler if it finds its state
// stale.
I think you have the best context regarding AppContext usage, if this comment is stale, then I can go ahead and use the same AppContext reference in both the controller and the scheduler.
There was a problem hiding this comment.
We will have to handle the case of changed role for the RCAScheduler. we can do the cleanup for a follow up PR. This is a bug fix, so I won't block this on the proper fix.
| if (rcaScheduler != null) { | ||
| rcaScheduler.updateAppContextWithMutedActions(actionsForMute); | ||
| } |
There was a problem hiding this comment.
We will have to handle the case of changed role for the RCAScheduler. we can do the cleanup for a follow up PR. This is a bug fix, so I won't block this on the proper fix.
Fixes #:
#505
Description of changes:
This PR fixes the bug where actions that were overridden to be disabled do not end up being marked as muted. The reason for this behavior is because the RCA controller, while updating the list of muted components, updates the graph nodes directly for RCAs, and deciders, it updates the list of muted actions in the AppContext.
However, the AppContext updated by the RcaController and the AppContext used by the RcaScheduler are different. The Scheduler only gets a snapshot of the AppContext during its initialization as it uses that AppContext to determine staleness, and never gets an updated AppContext till the scheduler is restarted.
This causes the update to go missing in the AppContext used by the RcaScheduler which eventually manifests as an unmuted action(when muting an action is involved). The fix basically exposes a new method from the scheduler to get the updated list of muted actions that it should pass down to the Actions through its AppContext.
Tests:
Unit tests, tested on docker with HeapSizeIncreaseAction.
If new tests are added, how long do the new ones take to complete
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.