RFC 5545 recur schedule support by grepsr · Pull Request #931 · HubSpot/Singularity

grepsr · 2016-03-02T15:27:36Z

To get over cron/quartzCron limitations for my own projects, I tried to add RFC5545 support.
https://tools.ietf.org/html/rfc5545#section-3.3.10

I've been running this on my testing server with great success.

The scheduling is much more easy to configure and dynamic with RFC5545 (things like start date, end date, odd schedules like every 37 hours or every 93 days works with this)

Have not had the time to work on the UI to make the new ScheduleType appear; but once this is reviewed, adding that would be trivial.

tpetr · 2016-03-02T15:35:42Z

 import com.hubspot.singularity.WebExceptions;
 import com.hubspot.singularity.config.SingularityConfiguration;
 import com.hubspot.singularity.data.history.DeployHistoryHelper;
+import org.dmfs.rfc5545.recur.RecurrenceRule;


can you sort these imports? if you're using Eclipse or IntelliJ, we have a formatter config here: https://github.com/HubSpot/Singularity/blob/master/eclipse/singularity_eclipse_formatter.xml

Code re-formatted.

Wow that re-formatted everything, perhaps I should just re-format the sections I added?

Yeah, just your sections sounds good. Thanks!

grepsr · 2016-03-03T14:55:16Z

Not sure why the travis check keeps failing, it compiles file on my machine. Any idea how to go from here?

tpetr · 2016-03-03T15:10:41Z

@grepsr the Travis output makes it look like there's a findbugs issue. How are you building it locally? Try running mvn clean verify to compile, test, package, and analyzes the code for possible bugs or style issues.

grepsr · 2016-03-03T15:11:57Z

Thanks, I'm using mvn clean package

I'll try that.

tpetr · 2016-03-04T15:05:40Z

+                        LOG.warn(msg);
+                        exceptionNotifier.notify(msg, ImmutableMap.of("taskId", taskId.toString()));
+                        return Optional.absent();
+                    }


could we refactor all this RFC5545 logic (lines 128-166) into its own class? it'd be nice to expose an interface similar to CronExpression.getNextValidTimeAfter() below

I actually thought about doing that too. I'll take care of it and commit

I'm not a Java guy, can you tell me where I should place the new RFC5545Schedule.java file into the folder structure? Should I put it inside com.hubspot.singularity folder inside the SingularityService module?

No worries -- I'd suggest putting it under com.hubspot.singularity.helpers

grepsr · 2016-03-04T16:46:38Z

I just refactored the code and moved everything into a helper class, much cleaner now.

grepsr · 2016-03-05T17:24:20Z

This finally worked on JDK8 on Travis, but still fails on JDK7 - not sure I understand why, any thoughts? Thanks

grepsr · 2016-03-09T15:08:17Z

Just wanted to check back on this and see if I could get any help getting this to work on JDK7?
JDK8 compiles great - really want to help move this feature into production branch.

grepsr · 2016-03-10T07:01:02Z

I just re-comitted the code without any change and it builds in both JDK7 and JDK8! I guess it was a Travis issue.

Where do we go from here?

tpetr · 2016-03-10T13:27:48Z

Awesome! Taking another look at this today. We'll need to add some unit tests since this change affects the operation of the scheduler.

grepsr · 2016-03-11T05:24:39Z

Great, please let me know if you need any help.

Also, I'm using this on production for the past 10 days and this is working great! I have about 200+ tasks scheduled and it schedules everything right on time.

grepsr · 2016-03-24T02:28:26Z

A note about this feature. Lately I've realised that the tasks are usually overdue by 5 minutes before running. I have a feeling its because of some default setting that triggers the check

Is there some global setting that can help here? Perhaps checkNewTasksEverySeconds?

ssalinas · 2016-03-24T12:46:48Z

So there are two things to check here (second one is most likely the issue since you mentioned things are overdue):

Are the tasks being put in the pending queue with the correct time? You can check the tasks page in the ui and filter for scheduled. These should have the time they were expected to run at. If those times are not what you expect, that would indicate an error in the scheduler (i.e. choosing the wrong time based on the changes in this PR).
Are you getting offers with enough resources? While the tasks are put in the pending queue with an expected time to run, Singularity can only start those tasks that are due to run when an offer comes in that has the appropriate resources. So, if you have a scheduled job that is due, but you aren't getting offers, or you are getting offers that don't have enough resources, that would cause the tasks to be overdue until an appropriate offer came in. If this is the case, you may need more resources in your cluster, or another running framework in the cluster may be hogging all the offers
Only other thing I can think of is if you have maxTasksPerOffer set too low. This will limit the number of tasks Singularity can schedule from a single offer, meaning you then have to wait for more offers to schedule the next tasks. By default, there is no limit, so if you don't have it set in your config this isn't the issue

Hope that helps

… metadata with a user

…nection

…failed; Healthcheck failure message will mention user app if connection refused; Healthcheck failure message will say 'task killed' instead of 'task failed'

This reverts commit c505905.

This reverts commit 719bf57.

…ainer environment variable

…local

…anywhere

grepsr · 2016-05-20T12:11:21Z

Hi there, we've fixed the merge conflicts for this, and pushed to latest branch.

ssalinas · 2016-06-08T13:57:44Z

closing in favor of #1071

tpetr reviewed Mar 2, 2016
View reviewed changes

tpetr reviewed Mar 4, 2016
View reviewed changes

ssalinas modified the milestone: 0.4.12 Mar 18, 2016

ssalinas modified the milestones: 0.5.0, 0.6.0 Apr 5, 2016

ssalinas and others added 9 commits May 13, 2016 11:58

remove @NotNull on optional

a440df7

add a mkdir -p for {{{ taskAppDirectory }}}

3a5248e

fix event not defined on deploy page

e336892

Fix stray '-' that was preventing task email from sending when it had…

d415f02

… metadata with a user

clean up run now cmd line input, allow multiple args

9d32277

also set shell false if pending task cmdLineArgs are set

0fdfa19

get latest horizon version

d11fc3c

large warning if singularity has no leader or leader has no mesos con…

5450dde

…nection

Healthcheck failure message won't show if task killed because deploy …

d70dd02

…failed; Healthcheck failure message will mention user app if connection refused; Healthcheck failure message will say 'task killed' instead of 'task failed'

subratbasnet and others added 24 commits May 20, 2016 15:34

no message

3b0006a

rfc 5545 schedule

d678e3e

rfc fixes

5eac765

get string representation

5c05488

singularity fixes

4f93777

recur

04ca7a9

singularity scheduler

3ccc964

check style fix

9e83aaa

fixes

121ddd9

hubspot fixes

17c831e

schedule fixes

8a41290

fixes

1ee6e16

joda time

677ac66

build

c6456ad

fixes

206b18b

re-format code

227e798

Add task id into the container environment variable

b6e6165

Revert "re-format code"

1c3bb38

This reverts commit c505905.

Revert "fixes"

7323191

This reverts commit 719bf57.

Refactor the scheduler code + move to class + added TASK_ID into cont…

0542c27

…ainer environment variable

fixes to solve maven build issue

0d4d891

forcing travis build again, compiles pretty well on jdk7 and jdk8 on …

0df87a0

…local

merged

0b254c6

removed checkSingularityWebhook method call because it doesn't exist …

d0ce129

…anywhere

ssalinas mentioned this pull request Jun 8, 2016

Reimplementation of RFC5545 Schedule #1071

Merged

ssalinas closed this Jun 8, 2016

ssalinas modified the milestone: 0.8.0 Jun 14, 2016

Conversation

grepsr commented Mar 2, 2016

Uh oh!

tpetr Mar 2, 2016

Choose a reason for hiding this comment

Uh oh!

grepsr Mar 2, 2016

Choose a reason for hiding this comment

Uh oh!

grepsr Mar 2, 2016

Choose a reason for hiding this comment

Uh oh!

tpetr Mar 4, 2016

Choose a reason for hiding this comment

Uh oh!

grepsr commented Mar 3, 2016

Uh oh!

tpetr commented Mar 3, 2016

Uh oh!

grepsr commented Mar 3, 2016

Uh oh!

tpetr Mar 4, 2016

Choose a reason for hiding this comment

Uh oh!

grepsr Mar 4, 2016

Choose a reason for hiding this comment

Uh oh!

tpetr Mar 4, 2016

Choose a reason for hiding this comment

Uh oh!

grepsr commented Mar 4, 2016

Uh oh!

grepsr commented Mar 5, 2016

Uh oh!

grepsr commented Mar 9, 2016

Uh oh!

grepsr commented Mar 10, 2016

Uh oh!

tpetr commented Mar 10, 2016

Uh oh!

grepsr commented Mar 11, 2016

Uh oh!

grepsr commented Mar 24, 2016

Uh oh!

ssalinas commented Mar 24, 2016

Uh oh!

grepsr commented May 20, 2016

Uh oh!

ssalinas commented Jun 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants