Skip to content

Commit

Permalink
Change the Job running mutex method
Browse files Browse the repository at this point in the history
The former mutex was causing contention in low schedule time Jobs, and causing other weird behaviours on deletes and updates due to that contention.

Also running above serf broadcast rate was causing queue length contention in serf itself.

Replacing it for a simple boolean check and skipping runs under 3 something seconds, that is the minimum broadcast time, allows to notify the user about the fact the execution skipped.
  • Loading branch information
Victor Castell committed Aug 24, 2019
1 parent bb5f01d commit ceda9d3
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions dkron/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package dkron
import (
"errors"
"fmt"
"sync"
"time"

"github.com/dgraph-io/badger"
Expand Down Expand Up @@ -95,7 +94,8 @@ type Job struct {
// Number of times to retry a job that failed an execution.
Retries uint `json:"retries"`

running sync.Mutex
// running indicates that the Run method is still broadcasting
running bool

// Jobs that are dependent upon this one will be run after this job runs.
DependentJobs []string `json:"dependent_jobs"`
Expand Down Expand Up @@ -199,8 +199,8 @@ func (j *Job) ToProto() *proto.Job {

// Run the job
func (j *Job) Run() {
j.running.Lock()
defer j.running.Unlock()
j.running = true
defer func() { j.running = false }()

// Maybe we are testing or it's disabled
if j.Agent != nil && j.Disabled == false {
Expand Down Expand Up @@ -321,9 +321,14 @@ func (j *Job) isRunnable() bool {
"job": j.Name,
"concurrency": j.Concurrency,
"job_status": j.Status,
}).Debug("scheduler: Skipping execution")
}).Info("job: Skipping concurrent execution")
return false
}

if j.running {
log.WithField("job", j.Name).
Warning("job: Skipping execution because last execution still broadcasting, consider increasing schedule interval")
}

return true
}

0 comments on commit ceda9d3

Please sign in to comment.