-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
Allow release of resources during task running. #2346
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b445be4
Add scheduler and task methods to control running resources.
riga 5701454
Enable debug notifications in test again.
riga 8fa2f4e
Prefer resources_running over resources in visualizer.
riga cb79e3a
Use decrease_running_resources instead of set_running_resouces.
riga f2620c2
Skip remote scheduler tests on travis.
riga e7967af
Merge branch 'master' into dynamicResources.
riga 036b813
Copy task resource dicts in scheduler.
riga 20e3033
Add test for decreasing task resources in to scheduler API tests.
riga d530d47
Enable running_resources test again on travis.
riga dc2a74a
Make task callbacks in TaskProcess dynamic.
riga c319bf1
Avoid hardcoded unpicklable_properties in task.
riga 2939a52
Refactor running resources tests.
riga 41c9b37
Merge branch 'master' into dynamicResources.
riga File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
# -*- coding: utf-8 -*- | ||
# | ||
# Copyright 2012-2015 Spotify AB | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
import os | ||
import time | ||
import signal | ||
import multiprocessing | ||
from contextlib import contextmanager | ||
|
||
from helpers import unittest, RunOnceTask | ||
|
||
import luigi | ||
import luigi.server | ||
|
||
|
||
class ResourceTestTask(RunOnceTask): | ||
|
||
param = luigi.Parameter() | ||
reduce_foo = luigi.BoolParameter() | ||
|
||
def process_resources(self): | ||
return {"foo": 2} | ||
|
||
def run(self): | ||
if self.reduce_foo: | ||
self.decrease_running_resources({"foo": 1}) | ||
|
||
time.sleep(2) | ||
|
||
super(ResourceTestTask, self).run() | ||
|
||
|
||
class ResourceWrapperTask(RunOnceTask): | ||
|
||
reduce_foo = ResourceTestTask.reduce_foo | ||
|
||
def requires(self): | ||
return [ | ||
ResourceTestTask(param="a", reduce_foo=self.reduce_foo), | ||
ResourceTestTask(param="b"), | ||
] | ||
|
||
|
||
class LocalRunningResourcesTest(unittest.TestCase): | ||
|
||
def test_resource_reduction(self): | ||
# trivial resource reduction on local scheduler | ||
# test the running_task_resources setter and getter | ||
sch = luigi.scheduler.Scheduler(resources={"foo": 2}) | ||
|
||
with luigi.worker.Worker(scheduler=sch) as w: | ||
task = ResourceTestTask(param="a", reduce_foo=True) | ||
|
||
w.add(task) | ||
w.run() | ||
|
||
self.assertEqual(sch.get_running_task_resources(task.task_id)["resources"]["foo"], 1) | ||
|
||
|
||
class ConcurrentRunningResourcesTest(unittest.TestCase): | ||
|
||
def get_app(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remove this in a follow-up? You don't need it anymore right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup! |
||
return luigi.server.app(luigi.scheduler.Scheduler()) | ||
|
||
def setUp(self): | ||
super(ConcurrentRunningResourcesTest, self).setUp() | ||
|
||
# run the luigi server in a new process and wait for its startup | ||
self._process = multiprocessing.Process(target=luigi.server.run) | ||
self._process.start() | ||
time.sleep(0.5) | ||
|
||
# configure the rpc scheduler, update the foo resource | ||
self.sch = luigi.rpc.RemoteScheduler() | ||
self.sch.update_resource("foo", 3) | ||
|
||
def tearDown(self): | ||
super(ConcurrentRunningResourcesTest, self).tearDown() | ||
|
||
# graceful server shutdown | ||
self._process.terminate() | ||
self._process.join(timeout=1) | ||
if self._process.is_alive(): | ||
os.kill(self._process.pid, signal.SIGKILL) | ||
|
||
@contextmanager | ||
def worker(self, scheduler=None, processes=2): | ||
with luigi.worker.Worker(scheduler=scheduler or self.sch, worker_processes=processes) as w: | ||
w._config.wait_interval = 0.2 | ||
w._config.check_unfulfilled_deps = False | ||
yield w | ||
|
||
@contextmanager | ||
def assert_duration(self, min_duration=0, max_duration=-1): | ||
t0 = time.time() | ||
try: | ||
yield | ||
finally: | ||
duration = time.time() - t0 | ||
self.assertGreater(duration, min_duration) | ||
if max_duration > 0: | ||
self.assertLess(duration, max_duration) | ||
|
||
def test_tasks_serial(self): | ||
# serial test | ||
# run two tasks that do not reduce the "foo" resource | ||
# as the total foo resource (3) is smaller than the requirement of two tasks (4), | ||
# the scheduler is forced to run them serially which takes longer than 4 seconds | ||
with self.worker() as w: | ||
w.add(ResourceWrapperTask(reduce_foo=False)) | ||
|
||
with self.assert_duration(min_duration=4): | ||
w.run() | ||
|
||
def test_tasks_parallel(self): | ||
# parallel test | ||
# run two tasks and the first one lowers its requirement on the "foo" resource, so that | ||
# the total "foo" resource (3) is sufficient to run both tasks in parallel shortly after | ||
# the first task started, so the entire process should not exceed 4 seconds | ||
with self.worker() as w: | ||
w.add(ResourceWrapperTask(reduce_foo=True)) | ||
|
||
with self.assert_duration(max_duration=4): | ||
w.run() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice code improvement :)