You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ariatosca.apache.org by mxmrlv <gi...@git.apache.org> on 2017/06/25 12:43:31 UTC
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
GitHub user mxmrlv opened a pull request:
https://github.com/apache/incubator-ariatosca/pull/163
introduced process grouping
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-285-Cancel-execution-may-leave-running-processes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/163.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #163
----
commit 4e20e999f6dc3a6db92e31980155506c56e19929
Author: max-orlov <ma...@gigaspaces.com>
Date: 2017-06-25T09:19:02Z
introduced process grouping
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124256836
--- Diff: aria/orchestrator/workflows/executor/process.py ---
@@ -125,10 +140,16 @@ def _execute(self, ctx):
env = self._construct_subprocess_env(task=ctx.task)
# Asynchronously start the operation in a subprocess
- subprocess.Popen(
- '{0} {1} {2}'.format(sys.executable, __file__, arguments_json_path),
+ proc = subprocess.Popen(
+ [
+ sys.executable,
+ os.path.expanduser(os.path.expandvars(__file__)),
+ os.path.expanduser(os.path.expandvars(arguments_json_path))
+ ],
env=env,
- shell=True)
+ preexec_fn=os.setsid)
--- End diff --
incompatible with windows
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124254946
--- Diff: aria/orchestrator/workflows/executor/process.py ---
@@ -113,9 +120,17 @@ def close(self):
self._server_socket.close()
self._listener_thread.join(timeout=60)
+ for task_id in self._tasks:
+ self.terminate(task_id)
+
+ def terminate(self, task_id):
+ task = self._tasks.get(task_id)
+ # The process might have managed to finished so it would not be in the tasks list
--- End diff --
fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca issue #163: introduced process grouping
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/incubator-ariatosca/pull/163
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/163
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124279733
--- Diff: aria/orchestrator/workflows/core/engine.py ---
@@ -74,9 +74,11 @@ def execute(self, ctx, resuming=False):
events.on_success_workflow_signal.send(ctx)
except BaseException as e:
# Cleanup any remaining tasks
- self._terminate_tasks(tasks_tracker.executing_tasks)
- events.on_failure_workflow_signal.send(ctx, exception=e)
- raise
+ try:
+ self._terminate_tasks(tasks_tracker.executing_tasks)
+ finally:
+ events.on_failure_workflow_signal.send(ctx, exception=e)
+ raise e
--- End diff --
just raise
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124280092
--- Diff: aria/orchestrator/workflows/core/engine.py ---
@@ -74,9 +74,11 @@ def execute(self, ctx, resuming=False):
events.on_success_workflow_signal.send(ctx)
except BaseException as e:
# Cleanup any remaining tasks
- self._terminate_tasks(tasks_tracker.executing_tasks)
- events.on_failure_workflow_signal.send(ctx, exception=e)
- raise
+ try:
+ self._terminate_tasks(tasks_tracker.executing_tasks)
+ finally:
--- End diff --
rmv finally
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124254651
--- Diff: aria/orchestrator/workflows/executor/process.py ---
@@ -25,6 +25,10 @@
# As part of the process executor implementation, subprocess are started with this module as their
# entry point. We thus remove this module's directory from the python path if it happens to be
# there
+from collections import namedtuple
+
--- End diff --
rmove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124257717
--- Diff: tests/orchestrator/workflows/executor/test_process_executor.py ---
@@ -71,10 +78,45 @@ def test_closed(self, executor, model):
executor.execute(MockContext(model, task_kwargs=dict(function='some.function')))
assert 'closed' in exc_info.value.message
+ def test_process_termination(self, executor, model, fs_test_holder):
+ argument = models.Argument.wrap('holder_path', fs_test_holder._path)
+ model.argument.put(argument)
+ ctx = MockContext(
+ model,
+ task_kwargs=dict(
+ function='{0}.{1}'.format(__name__, freezing_task.__name__),
+ arguments=dict(holder_path=argument)),
+ )
+
+ executor.execute(ctx)
+
+ @retrying.retry(retry_on_result=lambda r: r is False, stop_max_delay=60000, wait_fixed=500)
+ def wait_for_extra_process_id():
+ return fs_test_holder.get('subproc', False)
+
+ pids = [executor._tasks[ctx.task.id].proc.pid, wait_for_extra_process_id()]
+ assert any(p.pid == pid for p in psutil.process_iter() for pid in pids)
+ executor.terminate(ctx.task.id)
+
+ # Give a change to the processes to terminate
--- End diff --
chance
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124254271
--- Diff: aria/orchestrator/workflows/core/engine.py ---
@@ -66,13 +66,22 @@ def execute(self, ctx, resuming=False):
else:
time.sleep(0.1)
if cancel:
- events.on_cancelled_workflow_signal.send(ctx)
+ try:
+ self._terminate_tasks(tasks_tracker.executing_tasks)
+ finally:
+ events.on_cancelled_workflow_signal.send(ctx)
else:
events.on_success_workflow_signal.send(ctx)
except BaseException as e:
+ # Cleanup any remaining tasks
+ self._terminate_tasks(tasks_tracker.executing_tasks)
--- End diff --
try; finally
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124258268
--- Diff: aria/orchestrator/workflows/core/engine.py ---
@@ -66,13 +66,22 @@ def execute(self, ctx, resuming=False):
else:
time.sleep(0.1)
if cancel:
- events.on_cancelled_workflow_signal.send(ctx)
+ try:
--- End diff --
maybe only close?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #163: introduced process grouping
Posted by mxmrlv <gi...@git.apache.org>.
Github user mxmrlv commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/163#discussion_r124255084
--- Diff: aria/orchestrator/workflows/executor/process.py ---
@@ -113,9 +120,17 @@ def close(self):
self._server_socket.close()
self._listener_thread.join(timeout=60)
+ for task_id in self._tasks:
+ self.terminate(task_id)
+
+ def terminate(self, task_id):
+ task = self._tasks.get(task_id)
+ # The process might have managed to finished so it would not be in the tasks list
+ if task:
+ os.killpg(os.getpgid(task.proc.pid), signal.SIGKILL)
--- End diff --
try:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---