-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Pass deepCopy objects to the polling goroutines #1812
Conversation
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure this will fix the bug we talked about but I think it's definitely good to do no matter what. Down with mutable global state!
Actually as I'm looking at this harder, do we want to copy them here or at the point we actually launch the goroutines? keda/pkg/scaling/scale_handler.go Lines 104 to 105 in 347e897
I think the way this is now, both goroutines would get the same reference. |
Yeah, you are right, that's much better spot. Let me change that. |
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Zbynek Roubalik zroubali@redhat.com
Pass deepCopy of ScaledObject/ScaledJob objects to the scaleLoop polling goroutines it's a precaution to not have global objects shared between threads.
It should potentially help with the issue, which occurs if there is a big number of scalers running. Sometimes there are periodic panics in the json Marshall code that appeared to be related to passing the scaler objects to the background go routines and causing race conditions between fetching and serializing in different goroutines.
Checklist