[Feature] Flint query scheduler part1 - integrate job scheduler plugin #2834

noCharger · 2024-07-16T16:57:28Z

Description

Integrate job scheduler plugin as the first part of async query scheduler feature opensearch-project/opensearch-spark#416, which includes:

Introduce job scheduler plugin
1.1 Upgrade guava from 32.0.1-jre to 32.1.3-jre for all components
1.2 Consolidate on guava:failureaccess:1.0.2
1.3 Make SQLPlugin as a JobSchedulerExtension, add job-scheduler-spi as a dependency
1.4 Install job-scheduler plugin on integ test clusters, as well as the doc test cluster
Support async query scheduler feature in async-query module
2.1 Introduce job index .async-query-schedule
2.2 Add scheduler data model OpenSearchRefreshIndexJobRequest
2.3 Add scheduler job OpenSearchRefreshIndexJob
2.4 Add scheduler interface and implementation OpenSearchAsyncQueryScheduler

Consider seperate PRs for follow-up tasks:

Modify current use case to leverage on the new scheduler
Make job index a system index
Add a feature flag
Support streaming job migration
Make scheduler as a service
Add more integration test coverage, as well as BWC test
Migrate FlintStreamingJobHouseKeeperTask into the new scheduler

Sanity Test

On cluster bootstrap:

[2024-07-16T18:32:08,644][INFO ][o.o.j.JobSchedulerPlugin ] JobSweeper started listening to operations on index .async-query-scheduler
[2024-07-16T18:44:39,905][INFO ][o.o.j.s.JobScheduler     ] Scheduling job id my_job for index .async-query-scheduler .
[2024-07-16T18:44:39,906][INFO ][o.o.j.s.JobScheduler     ] Scheduling job id my_job2 for index .async-query-scheduler .

On path createJob:

[2024-07-16T18:34:00,161][INFO ][o.o.j.s.JobScheduler     ] Scheduling job id my_job for index .async-query-scheduler .
[2024-07-16T18:34:00,192][INFO ][o.o.s.s.s.OpenSearchAsyncQueryScheduler] Job : my_job  successfully created

[2024-07-16T18:46:44,009][INFO ][o.o.p.PluginsService     ] PluginService:onIndexModule index:[.opendistro-job-scheduler-lock/bHplaDQkTPGH_PbF97SdWw]
[2024-07-16T18:46:44,015][INFO ][o.o.c.m.MetadataCreateIndexService] [.opendistro-job-scheduler-lock] creating index, cause [api], templates [], shards [1]/[1]

On path removeJob:

[2024-07-16T18:29:18,292][INFO ][o.o.j.s.JobSweeper       ] Descheduling job my_job on index .async-query-scheduler
[2024-07-16T18:29:18,292][INFO ][o.o.j.s.JobScheduler     ] Descheduling jobId: my_job
[2024-07-16T18:29:18,337][INFO ][o.o.s.s.s.OpenSearchAsyncQueryScheduler] Job : my_job successfully deleted

Issues Resolved

#2832

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc 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.

codecov · 2024-07-17T00:40:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.28%. Comparing base (2c29a1a) to head (6f85b9a).
Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2834      +/-   ##
============================================
+ Coverage     94.60%   96.28%   +1.68%     
+ Complexity     5140     4904     -236     
============================================
  Files           508      467      -41     
  Lines         14473    13366    -1107     
  Branches        944      887      -57     
============================================
- Hits          13692    12870     -822     
+ Misses          740      463     -277     
+ Partials         41       33       -8

Flag	Coverage Δ
sql-engine	`96.28% <ø> (+1.68%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Louis Chu <[email protected]>

core/build.gradle

plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

async-query/src/main/java/org/opensearch/sql/spark/scheduler/OpenSearchAsyncQueryScheduler.java

penghuo · 2024-07-24T15:41:24Z

...src/main/java/org/opensearch/sql/spark/scheduler/model/OpenSearchRefreshIndexJobRequest.java

+  public static final String ENABLED_FIELD = "enabled";
+
+  // name is doc id
+  private final String jobName;


what is jobName? could u explain more name is doc id?

JobName is the required field for ScheduledJobParameter interface. The plan is to reuse it for the doc id.

penghuo · 2024-07-24T15:41:33Z

...src/main/java/org/opensearch/sql/spark/scheduler/model/OpenSearchRefreshIndexJobRequest.java

+
+  // name is doc id
+  private final String jobName;
+  private final String jobType;


what is jobType?

what is jobType?

This is for supporting different type of jobs, it should be moved to the parent class when abtract out the service layer

plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

penghuo · 2024-07-24T15:47:54Z

async-query/src/main/java/org/opensearch/sql/spark/scheduler/job/OpenSearchRefreshIndexJob.java

+ * and using singleton job runner to ensure we register a usable job runner instance to JobScheduler
+ * plugin.
+ */
+public class OpenSearchRefreshIndexJob implements ScheduledJobRunner {


+1. why add @Singleton

penghuo · 2024-07-24T15:50:19Z

...uery/src/test/java/org/opensearch/sql/spark/scheduler/job/OpenSearchRefreshIndexJobTest.java

+    jobRunner = OpenSearchRefreshIndexJob.getJobRunnerInstance();
+    jobRunner.setClient(null);
+    jobRunner.setClusterService(null);
+    jobRunner.setThreadPool(null);


why use setXXX? can we construct OpenSearchRefreshIndexJob(client, clusterService, threadpool)?

why use setXXX? can we construct OpenSearchRefreshIndexJob(client, clusterService, threadpool)?

Explained on comments here https://github.com/opensearch-project/sql/pull/2834/files/2b69b66166dcc0b022404d7424717b6503265bef#diff-b63487ae812d0eeb5c84aa6be4bca723773ffbe2f04056ff1807d31ca4d0e67eR23-R31

...uery/src/test/java/org/opensearch/sql/spark/scheduler/job/OpenSearchRefreshIndexJobTest.java

penghuo · 2024-07-24T15:54:11Z

...uery/src/test/java/org/opensearch/sql/spark/scheduler/OpenSearchAsyncQuerySchedulerTest.java

+import org.opensearch.sql.spark.scheduler.model.OpenSearchRefreshIndexJobRequest;
+import org.opensearch.threadpool.ThreadPool;
+
+public class OpenSearchAsyncQuerySchedulerTest {


nit: we can do test with TestCluster and verify the mapping, doc is expected also.

nit: we can do test with TestCluster and verify the mapping, doc is expected also.

Will in include this in part2. Thanks!

ykmr1224 · 2024-07-24T15:46:04Z

async-query/src/main/java/org/opensearch/sql/spark/scheduler/job/OpenSearchRefreshIndexJob.java

+  public static OpenSearchRefreshIndexJob INSTANCE = new OpenSearchRefreshIndexJob();
+
+  public static OpenSearchRefreshIndexJob getJobRunnerInstance() {
+    return INSTANCE;
+  }


With @Singleton annotation, Guice will use single instance when injecting to other classes. So we don't need to implement singleton pattern by ourselves.

With @Singleton annotation, Guice will use single instance when injecting to other classes. So we don't need to implement singleton pattern by ourselves.

getJobRunnerInstance is called during SQLPlugin instantiation, whereas injector = modules.createInjector(); is then called in createComponents. Please let me know how to apply Guice in this case. Thanks!

If injector is created after the instantiation, we cannot use @Singleton since that means we cannot rely on Guice to create the instance. But looks like the getJobRunnerInstance is called from OpenSearchAsyncQueryScheduler, which is instantiated in the same method. It should work even if we retrieve instance from the injector (retrieve instance of OpenSearchAsyncQueryScheduler from injector, and inject OpenSearchRefreshIndexJob into OpenSearchAsyncQueryScheduler by Guice).

getJobRunnerInstance is a static function, and it's actually called https://github.com/opensearch-project/sql/pull/2834/files/2b69b66166dcc0b022404d7424717b6503265bef#diff-b9ca34b799db4aa7b3c7dbccba65de6f539a5d3d6accf8a1c75e4c6893b9a9b8R267 during SQLPlugin instantiation

ykmr1224 · 2024-07-24T15:48:18Z

...src/main/java/org/opensearch/sql/spark/scheduler/model/OpenSearchRefreshIndexJobRequest.java

+  @Override
+  public String getName() {
+    return jobName;
+  }
+
+  public String getJobType() {
+    return jobType;
+  }
+
+  @Override
+  public Schedule getSchedule() {
+    return schedule;
+  }
+
+  @Override
+  public boolean isEnabled() {
+    return enabled;
+  }
+
+  @Override
+  public Instant getLastUpdateTime() {
+    return lastUpdateTime;
+  }
+
+  @Override
+  public Instant getEnabledTime() {
+    return enabledTime;
+  }
+
+  @Override
+  public Long getLockDurationSeconds() {
+    return lockDurationSeconds;
+  }
+
+  @Override
+  public Double getJitter() {
+    return jitter;
+  }


Can we use @Data or @Getter annotation on this class?

Can we use @Data or @Getter annotation on this class?

The fields must be overridden for the ScheduledJobParameter interface.

@Data and @Getter should be able to implement methods from interface. Did you try?

after removed

@Override public String getName() { return jobName; }

Tried both annotation

ykmr1224 · 2024-07-24T18:13:51Z

async-query/src/main/java/org/opensearch/sql/spark/scheduler/job/OpenSearchRefreshIndexJob.java

+  public static OpenSearchRefreshIndexJob INSTANCE = new OpenSearchRefreshIndexJob();
+
+  public static OpenSearchRefreshIndexJob getJobRunnerInstance() {
+    return INSTANCE;
+  }


If injector is created after the instantiation, we cannot use @Singleton since that means we cannot rely on Guice to create the instance. But looks like the getJobRunnerInstance is called from OpenSearchAsyncQueryScheduler, which is instantiated in the same method. It should work even if we retrieve instance from the injector (retrieve instance of OpenSearchAsyncQueryScheduler from injector, and inject OpenSearchRefreshIndexJob into OpenSearchAsyncQueryScheduler by Guice).

ykmr1224 · 2024-07-24T18:29:53Z

...src/main/java/org/opensearch/sql/spark/scheduler/model/OpenSearchRefreshIndexJobRequest.java

+  @Override
+  public String getName() {
+    return jobName;
+  }
+
+  public String getJobType() {
+    return jobType;
+  }
+
+  @Override
+  public Schedule getSchedule() {
+    return schedule;
+  }
+
+  @Override
+  public boolean isEnabled() {
+    return enabled;
+  }
+
+  @Override
+  public Instant getLastUpdateTime() {
+    return lastUpdateTime;
+  }
+
+  @Override
+  public Instant getEnabledTime() {
+    return enabledTime;
+  }
+
+  @Override
+  public Long getLockDurationSeconds() {
+    return lockDurationSeconds;
+  }
+
+  @Override
+  public Double getJitter() {
+    return jitter;
+  }


@Data and @Getter should be able to implement methods from interface. Did you try?

Signed-off-by: Louis Chu <[email protected]>

opensearch-trigger-bot · 2024-07-31T23:10:27Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.x
# Create a new branch
git switch --create backport/backport-2834-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 3daf64fbce5a8d29e846669689b3a7b12c5c7f07
# Push it to GitHub
git push --set-upstream origin backport/backport-2834-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-2834-to-2.x.

opensearch-project#2834) * [Feature] Flint query scheduler part1 - integrate job scheduler plugin Signed-off-by: Louis Chu <[email protected]> * Add comments Signed-off-by: Louis Chu <[email protected]> * Add unit test Signed-off-by: Louis Chu <[email protected]> * Remove test rest API Signed-off-by: Louis Chu <[email protected]> * Fix doc test Signed-off-by: Louis Chu <[email protected]> * Add more tests Signed-off-by: Louis Chu <[email protected]> * Fix IT Signed-off-by: Louis Chu <[email protected]> * Fix IT with security Signed-off-by: Louis Chu <[email protected]> * Improve test coverage Signed-off-by: Louis Chu <[email protected]> * Fix integTest cluster Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> * Update UT Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * clean up doc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> --------- Signed-off-by: Louis Chu <[email protected]> (cherry picked from commit 3daf64f)

#2834) (#2889) * [Feature] Flint query scheduler part1 - integrate job scheduler plugin Signed-off-by: Louis Chu <[email protected]> * Add comments Signed-off-by: Louis Chu <[email protected]> * Add unit test Signed-off-by: Louis Chu <[email protected]> * Remove test rest API Signed-off-by: Louis Chu <[email protected]> * Fix doc test Signed-off-by: Louis Chu <[email protected]> * Add more tests Signed-off-by: Louis Chu <[email protected]> * Fix IT Signed-off-by: Louis Chu <[email protected]> * Fix IT with security Signed-off-by: Louis Chu <[email protected]> * Improve test coverage Signed-off-by: Louis Chu <[email protected]> * Fix integTest cluster Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> * Update UT Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * clean up doc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> --------- Signed-off-by: Louis Chu <[email protected]> (cherry picked from commit 3daf64f)

opensearch-project#2834) * [Feature] Flint query scheduler part1 - integrate job scheduler plugin Signed-off-by: Louis Chu <[email protected]> * Add comments Signed-off-by: Louis Chu <[email protected]> * Add unit test Signed-off-by: Louis Chu <[email protected]> * Remove test rest API Signed-off-by: Louis Chu <[email protected]> * Fix doc test Signed-off-by: Louis Chu <[email protected]> * Add more tests Signed-off-by: Louis Chu <[email protected]> * Fix IT Signed-off-by: Louis Chu <[email protected]> * Fix IT with security Signed-off-by: Louis Chu <[email protected]> * Improve test coverage Signed-off-by: Louis Chu <[email protected]> * Fix integTest cluster Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> * Update UT Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix bwc test Signed-off-by: Louis Chu <[email protected]> * clean up doc test Signed-off-by: Louis Chu <[email protected]> * Resolve comments Signed-off-by: Louis Chu <[email protected]> * Fix UT Signed-off-by: Louis Chu <[email protected]> --------- Signed-off-by: Louis Chu <[email protected]>

noCharger force-pushed the feature-query-scheduler branch 2 times, most recently from b29ffc4 to 91bd2bc Compare July 17, 2024 00:18

noCharger force-pushed the feature-query-scheduler branch 2 times, most recently from 7224729 to 783a644 Compare July 17, 2024 01:43

noCharger self-assigned this Jul 17, 2024

noCharger added feature v2.16.0 Issues targeting release v2.16.0 labels Jul 17, 2024

noCharger force-pushed the feature-query-scheduler branch 4 times, most recently from e55762f to 3d70c7d Compare July 17, 2024 16:11

noCharger marked this pull request as ready for review July 17, 2024 19:42

noCharger requested review from ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vmmusings, Swiddis, penghuo, seankao-az, MaxKsyunz, Yury-Fridlyand, anirudha, forestmvey and acarbonetto as code owners July 17, 2024 19:42

noCharger added 2 commits July 24, 2024 08:17

Fix bwc test

b8a586e

Signed-off-by: Louis Chu <[email protected]>

clean up doc test

2b69b66

Signed-off-by: Louis Chu <[email protected]>

noCharger force-pushed the feature-query-scheduler branch from a004500 to 2b69b66 Compare July 24, 2024 15:18

penghuo reviewed Jul 24, 2024

View reviewed changes

ykmr1224 requested changes Jul 24, 2024

View reviewed changes

noCharger requested a review from ykmr1224 July 24, 2024 16:10

ykmr1224 requested changes Jul 24, 2024

View reviewed changes

noCharger requested a review from ykmr1224 July 24, 2024 18:48

noCharger added 2 commits July 24, 2024 12:17

Resolve comments

81c4c93

Signed-off-by: Louis Chu <[email protected]>

Fix UT

c59f70b

Signed-off-by: Louis Chu <[email protected]>

noCharger requested a review from penghuo July 25, 2024 16:41

ykmr1224 approved these changes Jul 25, 2024

View reviewed changes

penghuo approved these changes Jul 26, 2024

View reviewed changes

ykmr1224 added the backport 2.x label Jul 31, 2024

ykmr1224 merged commit 3daf64f into opensearch-project:main Jul 31, 2024
15 checks passed

opensearch-trigger-bot bot added the backport-failed label Jul 31, 2024

This was referenced Aug 1, 2024

[Manual Backport 2.x] Flint query scheduler part1 - integrate job scheduler plugin #2834 #2888

Closed

[Manual Backport 2.x][Feature] Flint query scheduler part1 - integrate job scheduler plugi… #2889

Merged

This was referenced Aug 7, 2024

Fix Observability CI workflow checks opensearch-project/dashboards-observability#2046

Merged

[Backport 2.x] Fix Observability CI workflow checks (#2046) opensearch-project/dashboards-observability#2054

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Flint query scheduler part1 - integrate job scheduler plugin #2834

[Feature] Flint query scheduler part1 - integrate job scheduler plugin #2834

noCharger commented Jul 16, 2024 •

edited

Loading

codecov bot commented Jul 17, 2024 •

edited

Loading

penghuo Jul 24, 2024

noCharger Jul 24, 2024

penghuo Jul 24, 2024

noCharger Jul 24, 2024

penghuo Jul 24, 2024

penghuo Jul 24, 2024

noCharger Jul 24, 2024

penghuo Jul 24, 2024

noCharger Jul 24, 2024 •

edited

Loading

ykmr1224 Jul 24, 2024

noCharger Jul 24, 2024

ykmr1224 Jul 24, 2024

noCharger Jul 24, 2024

ykmr1224 Jul 24, 2024

noCharger Jul 24, 2024

ykmr1224 Jul 24, 2024

noCharger Jul 24, 2024 •

edited

Loading

ykmr1224 Jul 24, 2024

ykmr1224 Jul 24, 2024

opensearch-trigger-bot bot commented Jul 31, 2024

[Feature] Flint query scheduler part1 - integrate job scheduler plugin #2834

[Feature] Flint query scheduler part1 - integrate job scheduler plugin #2834

Conversation

noCharger commented Jul 16, 2024 • edited Loading

Description

Sanity Test

Issues Resolved

Check List

codecov bot commented Jul 17, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noCharger Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noCharger Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jul 31, 2024

noCharger commented Jul 16, 2024 •

edited

Loading

codecov bot commented Jul 17, 2024 •

edited

Loading

noCharger Jul 24, 2024 •

edited

Loading

noCharger Jul 24, 2024 •

edited

Loading