-
Notifications
You must be signed in to change notification settings - Fork 251
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
Describe Dynflow scaling #1592
Describe Dynflow scaling #1592
Conversation
Out of the box, foreman ships with orchestrator and a single worker | ||
If you have Katello, you will get an additional worker for processing of the host queue | ||
``` | ||
root@modest-gator:~# ls -l /etc/foreman/dynflow/ |
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.
Nitpick: I'd probably just leave #
here.
root@modest-gator:/etc/foreman/dynflow# systemctl disable --now dynflow-sidekiq@worker-2 | ||
Removed /etc/systemd/system/multi-user.target.wants/dynflow-sidekiq@worker-2.service. | ||
|
||
root@modest-gator:/etc/foreman/dynflow# rm /etc/foreman/dynflow/worker-2.yml |
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.
I wonder why we don't actually ship 42 symlinks so users would not need to create or remove them and they could simply start and stop services. Overall I very much like this design, good work.
Anyway, I think you can leave a note that the file can be left there and won't hurt (I guess) until more instances are required.
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.
Btw 42 was just joke, 1-9 would be probably enough. We could even create a helper command: foreman-maintain dynflow scale-up --by 3
:-)
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.
The original idea was to have the installer to this, but it somehow fell through the cracks. Not having to do this by hand would be nice
|
||
``` | ||
root@modest-gator:~# cat /etc/foreman/dynflow/worker.yml | ||
:concurrency: 5 |
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.
Out of curiousity: why 5 and not 35? If we know it's thread safe, then we should probably put more work per process.
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.
IIRC the original all-in-one executor had 5 threads for the default queue, so in here we just copied it over.
Threads in ruby aren't exactly a silver bullet, so more threads doesn't always mean better performance. We need to find the right balance between the number of processes and number of threads per process.
I admit 5 may be too conservative, but I'd like to wait for the performance team to run some tests before we change it.
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.
Generally speaking, our actions (or whatever these are called in dynflow) are I/O bound so I would expect better performance from higher concurrency. Performance team should confirm. We should at least describe this down below what are the sane numbers here.
foreman 1785 1 0 11:32 ? 00:00:14 sidekiq 5.2.8 [0 of 5 busy] | ||
``` | ||
|
||
Next we change it to 15 |
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.
This is bad documentation honestly, we explain the process but not why you would want to do this and what are expected results. I know this is probably something our perf team should confirm, then let's resivist this later on.l
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.
This applies to this entire chapter. I'm afraid I don't have enough data to back any claims around why anyone should do this.
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.
Other than the hostname I am good.
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.
Thanks it's good enough.
Please submit a PR that adds this to 2.1 and nightly, otherwise it gets lost. |
### Scaling up | ||
Scaling up is pretty straightforward, especially if you want to only scale up what you have | ||
Here we use symbolic links to "share" the actual configuration among `worker`, `worker-1` and `worker-2` | ||
``` |
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.
You can use console highlighting for all of these
Opened a PR copying this over to 2.1 and nightly manuals #1594 |
Thanks @adamruzicka! |
Taken verbatim from
https://gist.github.com/adamruzicka/1991892ce22b18e030f9a4db95406319
CC @ares