Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Transform] Introduce TransformScheduler central service #84657

Merged
merged 11 commits into from
Jun 27, 2022
Merged

[Transform] Introduce TransformScheduler central service #84657

merged 11 commits into from
Jun 27, 2022

Conversation

przemekwitek
Copy link
Contributor

@przemekwitek przemekwitek commented Mar 4, 2022 •
edited
Loading

This PR introduces TransformScheduler class as a central place for scheduling transform tasks. The outline of how this mechanism works is:

  • There is a queue of transform tasks, sorted by their next_scheduled_time
  • On transform _start a transform task is added to the queue
  • Every schedulerFrequency (defaulting to 5s) the first task is being picked up from the queue and processed, i.e.: its listener is being triggered (indirectly triggering the indexer)
  • On transform _stop a transform task is removed from the queue and no longer processed
  • On transform failure (transient failure, to be precise), the task's next_scheduled_time is updated so that it is processed before it normally would. We are using exponential backoff scheme to calculate the appropriate delays
  • SchedulerEngine is no longer used

Additionally, this PR implements unit tests for TransformTaskParams and TransformContext classes.

The following work items are not covered by this PR. These will be handled separately, in the subsequent PRs:

  • configurable per-transform setting: numFailureRetries
  • support for infinite numFailureRetries
  • turning some of the irrecoverable errors into recoverable
  • increase maximum possible transform frequency

Relates to #85016

@przemekwitek przemekwitek force-pushed the transform_pipeline branch 2 times, most recently from 7d3cc9e to d374e9e Compare March 10, 2022 12:44
@przemekwitek przemekwitek force-pushed the transform_pipeline branch 5 times, most recently from a799c76 to 4560772 Compare March 17, 2022 08:26
@przemekwitek przemekwitek force-pushed the transform_pipeline branch 3 times, most recently from 4376066 to ab42380 Compare April 27, 2022 08:34
@przemekwitek przemekwitek force-pushed the transform_pipeline branch 2 times, most recently from cb2b596 to be0171a Compare May 4, 2022 10:04
@przemekwitek przemekwitek force-pushed the transform_pipeline branch 5 times, most recently from 54fa083 to 8a0ec3c Compare May 20, 2022 10:54
@przemekwitek
Copy link
Contributor Author

run elasticsearch-ci/part-2

@przemekwitek przemekwitek force-pushed the transform_pipeline branch 2 times, most recently from e7e109b to f8c33a8 Compare May 20, 2022 11:54
@przemekwitek przemekwitek removed the WIP label May 20, 2022
@przemekwitek przemekwitek marked this pull request as ready for review May 20, 2022 12:35
@hendrikmuhs hendrikmuhs self-requested a review May 30, 2022 14:41
hendrikmuhs
hendrikmuhs reviewed Jun 2, 2022
View reviewed changes
Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

add some first round comments and questions

...form/src/main/java/org/elasticsearch/xpack/transform/transforms/ConcurrentPriorityQueue.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
davidkyle
davidkyle reviewed Jun 7, 2022
View reviewed changes
...form/src/main/java/org/elasticsearch/xpack/transform/transforms/ConcurrentPriorityQueue.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformScheduler.java Outdated Show resolved Hide resolved
...form/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformSchedulerTests.java Outdated Show resolved Hide resolved
...form/src/main/java/org/elasticsearch/xpack/transform/transforms/ConcurrentPriorityQueue.java Outdated Show resolved Hide resolved
@przemekwitek przemekwitek mentioned this pull request Jun 7, 2022
Merged
@przemekwitek przemekwitek force-pushed the transform_pipeline branch 2 times, most recently from cdf95fe to 9300a47 Compare June 9, 2022 13:26
davidkyle
davidkyle reviewed Jun 23, 2022
View reviewed changes
Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

Looks good, I left some minor comments

...ain/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java Show resolved Hide resolved
...ain/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java Outdated Show resolved Hide resolved
...ava/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskQueue.java Outdated Show resolved Hide resolved
...ava/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskQueue.java Outdated Show resolved Hide resolved
...ava/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskQueue.java Outdated Show resolved Hide resolved
...rc/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduler.java Outdated Show resolved Hide resolved
...rc/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduler.java Outdated Show resolved Hide resolved
...rc/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduler.java Outdated Show resolved Hide resolved
hendrikmuhs
hendrikmuhs approved these changes Jun 24, 2022
View reviewed changes
Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

I have no further comments. LGTM

davidkyle
davidkyle approved these changes Jun 26, 2022
View reviewed changes
Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

@przemekwitek przemekwitek merged commit 9b3eb69 into elastic:master Jun 27, 2022
@przemekwitek przemekwitek deleted the transform_pipeline branch June 27, 2022 08:29
@hendrikmuhs hendrikmuhs mentioned this pull request Jul 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benwtrent benwtrent benwtrent left review comments

@hendrikmuhs hendrikmuhs hendrikmuhs approved these changes

@davidkyle davidkyle davidkyle approved these changes

Assignees
No one assigned
Labels
:ml/Transform Transform >refactoring Team:ML Meta label for the ML team v8.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@przemekwitek @elasticmachine @davidkyle @benwtrent @hendrikmuhs @craigtaverner @elasticsearchmachine @salvatore-campagna