Skip to content

fix issue #69#78

Open
vaporz wants to merge 2 commits intogocraft:masterfrom
vaporz:outdated_jobs
Open

fix issue #69#78
vaporz wants to merge 2 commits intogocraft:masterfrom
vaporz:outdated_jobs

Conversation

@vaporz
Copy link
Copy Markdown
Contributor

@vaporz vaporz commented Nov 7, 2017

fix issue #69 .

Comment thread worker.go Outdated
}
if jt, ok := w.jobTypes[job.Name]; ok {
if jt.StartingDeadline > 0 && job.ScheduledAt > 0 && job.ScheduledAt < jt.StartingDeadline {
logInfo("Outdated! StartingDeadline: %d, ScheduledAt: %d, Job: %s\n", jt.StartingDeadline, job.ScheduledAt, job)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't want to be logging info messages in the middle of a job processing loop - please remove this (it's not bad thing to have for running the tests).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment thread worker_pool.go
SkipDead bool // If true, don't send failed jobs to the dead queue when retries are exhausted.
MaxConcurrency uint // Max number of jobs to keep in flight (default is 0, meaning no max)
Backoff BackoffCalculator // If not set, uses the default backoff algorithm
StartingDeadline int64 // UTC time in seconds(time.Now().Unix()), the deadline for starting the job if it misses its scheduled time for any reason
Copy link
Copy Markdown
Collaborator

@shdunning shdunning Dec 13, 2017

Choose a reason for hiding this comment

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

Where is this set? Is the client code's responsibility?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is set when a job is registered in worker pool, before the pool is started.

@vaporz
Copy link
Copy Markdown
Contributor Author

vaporz commented Dec 13, 2017

@shdunning
Thanks for your comments.
Have changed code as your comments.

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.

2 participants