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

fix: Create copy of backoff struct #598

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Conversation

abatilo
Copy link
Contributor

@abatilo abatilo commented Feb 7, 2023

The backoff is created in interceptor/main.go and .Step() is a pointer receiver, so .Step() will mutate the underling number of .Steps within the struct. When you have 0 replicas, the default 500ms timeout of the Dial is hit which basically guarantees that you will end up calling .Step(). The value inside the the struct will never be reset, and so every single time you're scaling from 0, you're guaranteed to reduce the number of .Steps by 1 and this will never reset until the application itself has been restarted.

This change makes sure that every time we execute
DialContextWithRetry, we start with a fresh backoff which will start the .Steps at 5 since it's a clone of the original backoff. This backoff is available in the context of the function returned by DialContextWithRetry, which will be the one that gets decremented, and then will be garbage collected and we get a brand new 5 steps the next time we execute DialContextWithRetry.

Fixes #586

Signed-off-by: Aaron Batilo AaronBatilo@gmail.com

The backoff is created in `interceptor/main.go` and `.Step()` is a
pointer receiver, so `.Step()` will mutate the underling number of
`.Steps` within the struct. When you have 0 replicas, the default 500ms
timeout of the Dial is hit which basically guarantees that you will end
up calling `.Step()`. The value inside the the struct will never be
reset, and so every single time you're scaling from 0, you're guaranteed
to reduce the number of `.Steps` by 1 and this will never reset until
the application itself has been restarted.

This change makes sure that every time we execute
`DialContextWithRetry`, we start with a fresh `backoff` which will start
the `.Steps` at 5 since it's a clone of the original `backoff`. This
`backoff` is available in the context of the function returned by
`DialContextWithRetry`, which will be the one that gets decremented, and
then will be garbage collected and we get a brand new 5 steps the next
time we execute `DialContextWithRetry`.

Closes kedacore#586

Signed-off-by: Aaron Batilo <AaronBatilo@gmail.com>
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks a lot for the fix! ❤️

@JorTurFer JorTurFer enabled auto-merge (squash) February 7, 2023 18:07
@JorTurFer JorTurFer merged commit 8f90980 into kedacore:main Feb 7, 2023
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.

First request with 0 replicas sometimes takes ~20s
2 participants