Commit Graph

20 Commits (419335727286e828112274be5a30999dbab6f8e4)

Author SHA1 Message Date
Mikhail Mazurskiy b28a83a4cf
Migrate to GetControllerOf from meta/v1 package 2017-08-06 22:41:58 +10:00
Jacob Simpson 2c70e5df35 Manual changes. 2017-07-17 15:05:37 -07:00
Chao Xu 60604f8818 run hack/update-all 2017-06-22 11:31:03 -07:00
Chao Xu cde4772928 run ./root-rewrite-all-other-apis.sh, then run make all, pkg/... compiles 2017-06-22 11:30:52 -07:00
Chao Xu f4989a45a5 run root-rewrite-v1-..., compile 2017-06-22 10:25:57 -07:00
Anthony Yeh 4e1b07d9c2 CronJob: Check ControllerRef Name and UID in unit test. 2017-04-19 15:42:34 -07:00
Anthony Yeh d72eebd3fc CronJob: Add ControllerRef on all created Jobs. 2017-04-18 13:59:54 -07:00
Anthony Yeh 4c954d6dbb CronJob: Don't launch if DeletionTimestamp is set. 2017-04-18 13:59:31 -07:00
Maciej Szulik 7cba9d9c92 Issue 37166: remove everything from batch/v2alpha1 that is not new 2017-03-06 12:12:38 +01:00
peay 2b33de0684 Modify CronJob API to add job history limits, cleanup jobs in controller 2017-02-25 06:51:54 -05:00
deads2k 8a12000402 move client/record 2017-01-31 19:14:13 -05:00
Clayton Coleman 469df12038
refactor: move ListOptions references to metav1 2017-01-23 17:52:46 -05:00
Kubernetes Submit Queue 71c918d95c Merge pull request #40143 from peay/event-when-cronjob-cannot-start
Automatic merge from submit-queue (batch tested with PRs 40196, 40143, 40277)

Emit warning event when CronJob cannot determine starting time

**What this PR does / why we need it**:
In #39608, we've modified the error message for when a CronJob has too many unmet starting times to enumerate to figure out the next starting time. This makes it more "actionable", and the user can now set a deadline to avoid running into this. However, the error message is still only controller level AFAIK and thus not exposed to the user. From his perspective, there is no way to tell why the CronJob is not scheduling the next instance.

The PR adds a warning event in addition to the error in the controller manager's log. 

**Which issue this PR fixes**: This is an addition to PR #39608  regarding #36311.

**Special notes for your reviewer**: cc @soltysh 

**Release note**:

```release-note
```
2017-01-23 05:39:03 -08:00
peay 6b5c8f1d2f Emit warning when CronJob cannot determine starting time 2017-01-18 15:33:32 -05:00
Clayton Coleman 9a2a50cda7
refactor: use metav1.ObjectMeta in other types 2017-01-17 16:17:19 -05:00
Kubernetes Submit Queue 9d2fce7c22 Merge pull request #39608 from peay/cronjob-too-many-times-to-list
Automatic merge from submit-queue

Do not list CronJob unmet starting times beyond deadline

**What this PR does / why we need it**:

See #36311. `getRecentUnmetScheduleTimes` gives up after 100 unmet times to avoid wasting too much CPU or memory generating all the times, as it generates them sequentially.

When concurrency is forbidden, this is conceptually un-necessary: we only need the last unmet start time. This suggests that when concurrency is forbidden, we could generate times by going backward in time from now. This is not very practical as CronJob currently relies on a package that only provides `Next` and no `Prev`. Hand-cooking a `Prev` does not seem like a good idea. I could submit a PR to the cron library to add a `Prev` method, and use that when concurrency is forbidden through something like `getLastUnmetScheduleTime`. This would be `O(1)` and there would be no limit involved.

(edit: actually, even for the other concurrency settings, we only start the last unmet start times -- there is a `TODO` in the controller to actually start all of them, but that is not implemented at the moment. This means the solution would apply, at least temporarily, to all concurrency settings).

cc @soltysh what do you think?

In the meantime, I would suggest to do something simple. Currently, the user has no way to configure anything to ensure that his CronJob will not get stuck if one job takes more that 100 unmet times.

 `getRecentUnmetScheduleTimes` starts with an initial time corresponding to the last start (or to the creation of the CronJob, if nothing has started yet). However, when `StartingDeadlineSeconds` is set, the controller will not start anything that is older than the deadline, so if the last start is way beyond the deadline, we are generating potentially lots of unmet start times that will not be considered by the scheduler for scheduling anyway.

Consider a job running every minute, where the last instance has taken 120 minutes. This means there are more than 100 unmet times when we start counting from the last start time.

**The PR makes `getRecentUnmetScheduleTimes` only consider times that do not fall beyond the deadline.** Here, the CronJob can be configured with a `StartingDeadlineSeconds` of, say, 10 minutes. After the 120min job has run, `getRecentUnmetScheduleTimes` will only consider the times in the last 10 minutes from now, and will not get stuck.

As a side note on the max. number of unmet times to use as limits in terms of CPU used by the controller: I have run a quick benchmark on my i7 mac. Schedules corresponding to "once a week" tend to be more expensive to generate unmet times for. Just FYI.

```
+--------------+---------------+--------------+
|   SCHEDULE   | MISSED STARTS |    TIMING    |
+--------------+---------------+--------------+
| */1 * * * ?  |           100 | 383.645µs    |
| */30 * * * ? |           100 | 354.765µs    |
| 30 1 * * ?   |           100 | 1.065124ms   |
| 30 1 * * 0   |           100 | 1.80034ms    |
| */1 * * * ?  |           500 | 1.341365ms   |
| */30 * * * ? |           500 | 1.814441ms   |
| 30 1 * * ?   |           500 | 8.475012ms   |
| 30 1 * * 0   |           500 | 10.020613ms  |
| */1 * * * ?  |          1000 | 2.551697ms   |
| */30 * * * ? |          1000 | 4.075813ms   |
| 30 1 * * ?   |          1000 | 17.674945ms  |
| 30 1 * * 0   |          1000 | 19.149324ms  |
| */1 * * * ?  |         10000 | 25.725531ms  |
| */30 * * * ? |         10000 | 87.520022ms  |
| 30 1 * * ?   |         10000 | 174.29216ms  |
| 30 1 * * 0   |         10000 | 196.565748ms |
+--------------+---------------+--------------+
```

using

```.go
package main

import (
    "fmt"
    "time"
    "os"
    "strconv"

    "github.com/robfig/cron"
    "github.com/olekukonko/tablewriter"
)

func timeSchedule(schedule string, iterations int) (time.Duration) {
    sched, err := cron.ParseStandard(schedule)

    if err != nil {
        panic(fmt.Sprintf("Unparseable schedule: %s", err))
    }

    start := time.Now()
    t := time.Now()

    for i := 1; i <= iterations; i++ {
        t = sched.Next(t)
    }

    return time.Since(start)
}

func main() {
    table := tablewriter.NewWriter(os.Stdout)
    table.SetHeader([]string{"Schedule", "Missed starts", "Timing"})

    schedules := []string{"*/1 * * * ?", "*/30 * * * ?", "30 1 * * ?", "30 1 * * 0"}
    iteration_nums := []int{100, 500, 1000, 10000}

    for _, iterations := range iteration_nums {
        for _, schedule := range schedules {
            table.Append([]string{schedule,
                                  strconv.Itoa(iterations),
                                  timeSchedule(schedule, iterations).String()})
        }
    }
    table.Render()
}
```

**Which issue this PR fixes**: fixes #36311

**Special notes for your reviewer**:

**Release note**:

```release-note
```
2017-01-17 00:41:45 -08:00
peay d141a43d86 Do not list CronJob unmet starting times beyond deadline 2017-01-15 12:29:20 -05:00
deads2k 6a4d5cd7cc start the apimachinery repo 2017-01-11 09:09:48 -05:00
peay d8d69d1a36 Remove jobs that do not exist from active list of CronJob 2017-01-03 23:12:50 +00:00
gmarek 770e1c289a Change 'controller.go' filenames to more meaningfull ones 2016-12-05 09:12:22 +01:00