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.

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

feature: add optional parameter depends_on attribute to start dependent processes consecutively #1449

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

michaelhammann
Copy link

@michaelhammann michaelhammann commented Jul 16, 2021

In this pull request I implemented an optional process parameter depends_on. This optional depends_on option is used to only start a process once the required process reached the RUNNING state, or else start all dependee processes first.
Features:

  • Dependency cycles will be detected before the start of supervisor.
  • A timeout is thrown (spawnerror) if a process stays in STARTING for too long (default 60 seconds). If a process does not make it to RUNNING before the timeout is reached, all processes left in the dependee queue will be set to FATAL. This time can be set with the optional process parameter spawn_timeout.
  • The depends_on parameter can be used with multiple dependencies which are seperated by a whitespace. If the process is part of a group the group name also has to be added before a colon (:).
  • This feature (depends_on) is especially useful in combination with runningregex attribute feature: add support for runningregex to set RUNNING state accordingly #1446 we always use this instead of startsecs to know when a process is actually ready and running

In the following there are two minimal examples:

##################

In the following minimal example, "Hello World!" is printed.
[group:examplegroup]
programs=world

[program:hello]
command = /bin/sh -c "echo Hello"
autostart=false
autorestart=false
startsecs=0
stdout_logfile = ./log/hello_world.log
stderr_logfile = ./log/hello_world_err.log

[program:world]
command = /bin/sh -c "echo World"
autostart=false
autorestart=false
startsecs=0
stdout_logfile = ./log/hello_world.log
stderr_logfile = ./log/hello_world_err.log

[program:start_hello_world]
command = /bin/sh -c "echo !"
depends_on=hello examplegroup:world
autostart=true
autorestart=false
stdout_logfile = ./log/hello_world.log
stderr_logfile = ./log/hello_world_err.log
startsecs=0

###############

[program:foo]
command = /bin/sh -c "sleep 2 && echo foo (depends on bar) is running at && date && sleep 10"
autostart=true
autorestart=false
startsecs=0
stdout_logfile = ./logs/%(program_name)s_stdout.log
stderr_logfile = ./logs/%(program_name)s_stderr.log.log
runningregex=foo*is running at
depends_on:bar

[program:bar]
command = /bin/sh -c "sleep 80"
autostart=false
autorestart=false
startsecs=70
stdout_logfile = ./logs/%(program_name)s_stdout.log
stderr_logfile = ./logs/%(program_name)s_stderr.log
spawn_timeout=71

# keep track of RUNNING childs
running_childs = set()
# wait/loop until all childs are running
while set(self.config.depends_on.keys()) != running_childs:
Copy link

@alexsilva alexsilva Sep 13, 2021

Choose a reason for hiding this comment

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

The proposal looks good but code execution needs to be asynchronous. This code will stop the main supervisor loop. https://github.com/Supervisor/supervisor/blob/master/supervisor/supervisord.py#L174

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @alexsilva for the review. Stopping the main supervisor loop is not a good idea. I changed the implementation to use the rpcinterface also for spawning dependee processes when using autostart=true. However, there might be better ways to use asyncronous calls, if you have any ideas on how to approve this any help is appreciated. Additionally there was a bug that spawn errors upon any dependee process were not properly detected. This should be fixed now.

Copy link
Author

@michaelhammann michaelhammann Oct 5, 2021

Choose a reason for hiding this comment

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

I was able to implement the depends_on flag without stopping the main supervisor loop. I appreciate any feedback or thoughts from your side.

@samssann
Copy link

samssann commented Dec 7, 2021

Any update on when this could be added to a release?

@michaelhammann michaelhammann force-pushed the feature-add-depends_on-option branch 2 times, most recently from e97fe57 to d1a8866 Compare April 13, 2022 13:26
@michaelhammann
Copy link
Author

I rebased to the current master and fixed the existing unit tests. Hopefully this will be merged at some point. Would also be great to get a review.

Provide a meaningfull log output when a dependent process does not start and improve algorithm
@lfvjimisola
Copy link

@alexsilva Any chance that this will be or should we opt for an ad-hoc and less elegant solution?

@michaelhammann
Copy link
Author

michaelhammann commented Nov 22, 2022

@alexsilva Any chance that this will be or should we opt for an ad-hoc and less elegant solution?

If we want this feature to be added fast, one approach could be to try to implement it as a plugin, similar to what has been mentioned regarding a different feature request here: #1446 (comment) .Unfortunately I did not have time to have to have a closer look into creating this as a plugin yet. Happy to have a talk about it.

@LorenzoRogai
Copy link

@michaelhammann is it possible to do the same for shutdown order?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants