Adds a delay parameter to the job scheduler #61

downsrob · 2021-09-15T00:15:35Z

Signed-off-by: Clay Downs [email protected]

Description

Adds a delay parameter to the job scheduler which allows plugins to put off execution time by a fixed number of milliseconds. For example, an interval schedule which starts at 10 AM with a delay of 3 minutes and an interval of 7 minutes would run at 10:03 AM, then 10:10 AM, then 10:17 AM and so on. This parameter is being added to enable plugins to easily add a delay to job schedules without parsing and modifying the user defined interval or cron schedule.

This index state management PR depends on this job scheduler change to add the delay feature to ISM rollup jobs.

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Robert Downs <[email protected]>

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/CronSchedule.java

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/IntervalSchedule.java

Signed-off-by: Clay Downs <[email protected]>

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/Schedule.java

Signed-off-by: Clay Downs <[email protected]>

DEVELOPER_GUIDE.md

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/CronSchedule.java

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/IntervalSchedule.java

Signed-off-by: Clay Downs <[email protected]>

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/IntervalSchedule.java

adityaj1107 · 2021-09-28T21:33:17Z

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/IntervalSchedule.java

+        if (delta >= 0) {
+            long remainingScheduleTime = intervalInMillis - (delta % intervalInMillis);
+            return Duration.of(remainingScheduleTime, ChronoUnit.MILLIS);
+        } else {
+            return Duration.ofMillis(enabledTimeEpochMillis - currentTime.toEpochMilli());
+        }


Same comment on Ternary operators! Simplification of code increases its readability.

I am also a big fan of ternary operators! But I try to only use them in simple cases, and I really try to not include anything that takes much thought. I agree with you on the other cases but I will leave this one as an if/else for that reason

adityaj1107 · 2021-09-28T21:34:57Z

DEVELOPER_GUIDE.md

@@ -49,7 +49,7 @@ Then you will find the built artifact located at `build/distributions` directory
 ## Install
 Once you have built the plugin from source code, run
 ```bash
-opensearch-plugin install file://${PLUGIN_ZIP_FILE_PATH}
+opensearch-plugin install file:///path/to/target/releases/opensearch-job-scheduler-<version>.zip


What are your thoughts on adding the example path to target?

I don't like this very much, but this follows another plugin: https://github.com/opensearch-project/security-dashboards-plugin/blob/ad6e723d4710b91cbcf1f0ec543b6ec24b976c53/DEVELOPER_GUIDE.md and this change lets the lychee link checker succeed

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/CronSchedule.java

spi/src/test/java/org/opensearch/jobscheduler/spi/schedule/IntervalScheduleTests.java

adityaj1107 · 2021-09-28T21:53:20Z

I have taken a cursory pass! I have minor comments. If this is a blocker for a release, I can approve it. O/w we can address these comments and get this one into production then.

Signed-off-by: Clay Downs <[email protected]>

* Adds delay parameter to job scheduler * Adds tests for job scheduler delay parameter * Changes test and build workflow to 1.1 and corrects links Signed-off-by: Clay Downs <[email protected]>

* Adds delay parameter to job scheduler * Adds tests for job scheduler delay parameter Signed-off-by: Clay Downs <[email protected]>

* Enabled automated license header checks (#36) 1. Re-enabled the 'licenseHeaders' check in the build.gradle file. 2. Added an IntelliJ copyright profile to auto-generate the SPDX license header. Signed-off-by: Ketan Verma <[email protected]> * Release 1.0.0.0 (#40) Signed-off-by: bowenlan-amzn <[email protected]> * Bumping job-scheduler to build with OpenSearch(main) 1.1.0 (#44) * Bumping job-scheduler to build with OpenSearch(main) 1.1.0 Signed-off-by: Sarat Vemulapalli <[email protected]> * Updating docs link to our documentation website. Signed-off-by: Sarat Vemulapalli <[email protected]> * Using 1.1 snapshot version for OpenSearch (#48) Signed-off-by: Vacha <[email protected]> * Use standard snapshot build settings and OpenSearch 1.x. (#49) Signed-off-by: dblock <[email protected]> * Add `Getting Started` to Readme (#50) * added Getting Started section * Adds a delay parameter to the job scheduler (#61) * Adds delay parameter to job scheduler * Adds tests for job scheduler delay parameter * Changes test and build workflow to 1.1 and corrects links Signed-off-by: Clay Downs <[email protected]> * Updates job scheduler version to 1.2 (#68) * Updates job scheduler version to 1.2 and uses Maven for 1.2 dependencies Signed-off-by: Clay Downs <[email protected]> * Publish MD5 and SHA1 signatures. (#71) * Publish MD5 and SHA1 signatures. Signed-off-by: dblock <[email protected]> * Also publish 256 and 512 checksums. Signed-off-by: dblock <[email protected]> * Fixes possible negative jitter (#78) Signed-off-by: Drew Baugher <[email protected]> Co-authored-by: Ketan Verma <[email protected]> Co-authored-by: Bowen Lan <[email protected]> Co-authored-by: Sarat Vemulapalli <[email protected]> Co-authored-by: Vacha <[email protected]> Co-authored-by: Daniel Doubrovkine (dB.) <[email protected]> Co-authored-by: Sriram <[email protected]> Co-authored-by: Clay Downs <[email protected]> Co-authored-by: Drew Baugher <[email protected]>

* Adds delay parameter to job scheduler * Adds tests for job scheduler delay parameter * Changes test and build workflow to 1.1 and corrects links Signed-off-by: Clay Downs <[email protected]>

downsrob added 2 commits September 8, 2021 20:56

Adds delay parameter to job scheduler

5ad1431

Signed-off-by: Robert Downs <[email protected]>

Adds tests for job scheduler delay parameter

566e2cd

Signed-off-by: Robert Downs <[email protected]>

downsrob mentioned this pull request Sep 15, 2021

Adds implementation for the delay feature in rollup jobs opensearch-project/index-management#147

Merged

1 task

thalurur reviewed Sep 15, 2021

View reviewed changes

downsrob added 5 commits September 15, 2021 13:58

Makes the job scheduler XContent backwards compatible

c7fea9e

Signed-off-by: Clay Downs <[email protected]>

Updates job-scheduler version to 1.2 and removes delay getter/setter

8a3eef0

Signed-off-by: Clay Downs <[email protected]>

Changes test and build workflow to 1.1 and corrects links

743820a

Signed-off-by: Clay Downs <[email protected]>

Adds a link exception to link checker

bf417de

Signed-off-by: Clay Downs <[email protected]>

Modifies the link checker exclusion

14e9dc7

Signed-off-by: Clay Downs <[email protected]>

zengyan-amazon reviewed Sep 17, 2021

View reviewed changes

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/Schedule.java Outdated Show resolved Hide resolved

downsrob added 2 commits September 20, 2021 11:19

Removes delay setter, makes other schedule fields public

071bbd4

Signed-off-by: Clay Downs <[email protected]>

Sets gradle.properties version to 1.1

659336f

Signed-off-by: Clay Downs <[email protected]>

thalurur reviewed Sep 21, 2021

View reviewed changes

DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved

thalurur reviewed Sep 21, 2021

View reviewed changes

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/CronSchedule.java Outdated Show resolved Hide resolved

thalurur reviewed Sep 21, 2021

View reviewed changes

spi/src/main/java/org/opensearch/jobscheduler/spi/schedule/IntervalSchedule.java Outdated Show resolved Hide resolved

downsrob added 2 commits September 22, 2021 11:47

Fixes plugin install doc, removes delay setter

4a72993

Signed-off-by: Clay Downs <[email protected]>

Makes hashCode backwards compatible

0eab490

Signed-off-by: Clay Downs <[email protected]>

thalurur previously approved these changes Sep 27, 2021

View reviewed changes

adityaj1107 reviewed Sep 28, 2021

View reviewed changes

Reformats schedule classes, adds error messages to tests

472f0ec

Signed-off-by: Clay Downs <[email protected]>

downsrob dismissed thalurur’s stale review via 472f0ec September 28, 2021 23:37

thalurur approved these changes Sep 29, 2021

View reviewed changes

adityaj1107 approved these changes Sep 29, 2021

View reviewed changes

downsrob merged commit 2450767 into opensearch-project:main Sep 29, 2021

downsrob deleted the delay-feature branch September 29, 2021 16:48

downsrob mentioned this pull request Oct 16, 2021

Adds a delay parameter to the job scheduler (#61) #74

Merged

5 tasks

downsrob added a commit that referenced this pull request Oct 19, 2021

Adds a delay parameter to the job scheduler (#61) (#74)

9dbea56

* Adds delay parameter to job scheduler * Adds tests for job scheduler delay parameter Signed-off-by: Clay Downs <[email protected]>

dbwiddis mentioned this pull request Feb 8, 2023

[BUG] Unit test failure - testNextTimeToExecution FAILED #58

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a delay parameter to the job scheduler #61

Adds a delay parameter to the job scheduler #61

downsrob commented Sep 15, 2021 •

edited

Loading

adityaj1107 Sep 28, 2021

downsrob Sep 28, 2021

adityaj1107 Sep 28, 2021

downsrob Sep 28, 2021

adityaj1107 commented Sep 28, 2021

Adds a delay parameter to the job scheduler #61

Adds a delay parameter to the job scheduler #61

Conversation

downsrob commented Sep 15, 2021 • edited Loading

Description

Check List

adityaj1107 Sep 28, 2021

Choose a reason for hiding this comment

downsrob Sep 28, 2021

Choose a reason for hiding this comment

adityaj1107 Sep 28, 2021

Choose a reason for hiding this comment

downsrob Sep 28, 2021

Choose a reason for hiding this comment

adityaj1107 commented Sep 28, 2021

downsrob commented Sep 15, 2021 •

edited

Loading