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

Cron scaler may have jitter #3854

Closed
zou2699 opened this issue Nov 14, 2022 · 0 comments · Fixed by #3838
Closed

Cron scaler may have jitter #3854

zou2699 opened this issue Nov 14, 2022 · 0 comments · Fixed by #3838
Labels
bug Something isn't working

Comments

@zou2699
Copy link
Contributor

zou2699 commented Nov 14, 2022

Report

We use the following expression to expand the capacity from the end of 8:00 am on the last day of each month to 8:00 pm on the 1st of the next month

start: "0 8 L * *"
end: "0 20 1 * *"

But the preexisting dependency library cron does not support L (the last day of month), see issue robfig/cron#464

So we use the following expression instead

start:"0 8 28,29,30,31 * *"
end: "0 20 1 * *"

But currently cronScaler.IsActive may trigger an issue like

     // if  execTime=2022.10.30 07:59:59.999
      nextStartTime, startTimecronErr := getCronTime(location, s.metadata.start)
      // got nextStartTime=2022.10.30 08:00:00.000
       ......
      nextEndTime, endTimecronErr := getCronTime(location, s.metadata.end)
     // got nextEndTime=2022.11.01 20:00:00.000
      ......
     currentTime := time.Now().Unix()
     // got currentTime=2022.10.30 08:00:00.111
     switch {
       case nextStartTime < nextEndTime && currentTime < nextStartTime:
            return false, nil
       // will use this case, return true, will scale up pod
      // but in the next cycle,  will retrun false       
       case currentTime <= nextEndTime:
           return true, nil
       default:
           return false, nil
}

this will cause an unexpected scale up event

when we get the currentTime before getting nextStartTime in cronScaler.IsActive, we can avoid this mistake

Expected Behavior

As can be seen from the above,if execTime=2022.10.30 07:59:59.999, these variables should appear as follows
currentTime=2022.10.30 08:00:00.111
nextStartTime=2022.10.30 08:00:00.000
nextEndTime=2022.11.01 20:00:00.000

the switch paragraph should enter the first case,and return false, because currentTime should always less than nextStartTime

Actual Behavior

As can be seen from the above,if execTime=2022.10.30 07:59:59.999, these variables may appear as follows
nextStartTime=2022.10.30 08:00:00.000
nextEndTime=2022.11.01 20:00:00.000
currentTime=2022.10.30 08:00:00.111

the switch paragraph will enter the second case,and return true, because currentTime is greater than nextStartTime

Steps to Reproduce the Problem

Unlikely to reproduce,Because time conditions are difficult to reproduce. but this problem may occur at some time.

FIX: #3838

Logs from KEDA operator

example

KEDA Version

No response

Kubernetes Version

No response

Platform

No response

Scaler Details

No response

Anything else?

No response

@zou2699 zou2699 added the bug Something isn't working label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant