Revert many changes that break systemd

[jclusso]

I'm the original author of the sidekiq multi process code. I think I have some useful insights into what's happened since my PR was merged.

In #296, an issue was opened about not supporting older versions of systemd that did not support multiple process service naming with the @ symbol. The solution for this was to make it so that when sidekiq_processes is set to 1, it uses the sidekiq_service_unit_name without the @.

However, the changes implemented in #297 changed the sidekiq_service_file_name method to never have the @ sign, and they changed the sidekiq_service_unit_name method to have some conditional logic that doesn't always work.

As a result of the changes in #297, #298 was created, which led to #299. #299 modified the sidekiq:install task's functionality in an incorrect way, changing it to create multiple sidekiq@[number].service files, which is not how systemd multiple process support is supposed to work. The purpose of the @ sign in systemd service file names is that everything after the @ is passed in as an argument. Therefore, the correct service file name for a multiple process service is
"sidekiq@.service". This allows for the full functionality of systemd service arguments. For example, ranges are supported, so you can run a command like systemctl restart sidekiq@{1..3} and that single command will execute for all 3 processes. Even though creating multiple files does technically work, I don't believe it is correct, and there is no benefit to having multiple identical service files when you only need one.

In order to address the issues created by the changes mentioned above, #300 was created. However, it did not fix everything, which led to #302 and #304, which are further attempts to fix issues created by these changes. Ultimately, you end up with a whole series of changes attempting to patch issues going back to #296. In my opinion, things would be simpler and easier if the code went back to behaving closer to how things were around #296.

The code in this pull request does that. When sidekiq_processes is set to one, the systemd unit file will be named "sidekiq.service" and all commands will run on the service named "sidekiq". When sidekiq_processes is greater than one, the systemd unit file will be named "sidekiq@.service" and all commands will run either a systemd multi-process command, like sidekiq@{1..3}, or, in the case of the restart command, it will run on each numbered process in turn (first sidekiq@1, then sidekiq@2, etc.).

Finally, in regards to issues around backward compatibility, you can simply run sidekiq:uninstall before upgrading and sidekiq:install after upgrading. This makes more sense than over complicating the code.

[legendetm]

I also tested this through and this was working as expected.

For me only sidekiq:uninstall was not working, because the user does not have sudo permission, but this is not directly related with this PR.

I was also trying to setup multiprocess Sidekiq services today and and it was not working as expected. Started to read about how systemd works @ and tried to fix the same problem.

Thankfully I noticed your PR that resolved the multiple process problem. Thank you.

Some resources that I was reading to understand how the systemd with @ is working

• https://www.freedesktop.org/software/systemd/man/systemd.service.html#Service%20Templates
• https://superuser.com/questions/1313119/how-to-manage-a-systemd-service-which-needs-multiple-processes
• Google search systemd start multiple processes with @

[jclusso]

@legendetm I will be honest I don't know much about the permissions thing but I know this option exists sidekiq_service_unit_user which maybe can help. I just kept existing stuff like that in place when I added multi process support.

[legendetm]

@jclusso it would be nice to update the documentation that before changing the sidekiq_processes count, you should run sidekiq:uninstall and after the changes sidekiq:install again. This is needed when upgrading from 1 process to multiple processes or when downgrading processes. When you already have multiple processes (2 or more) and adding more processes, then just sidekiq:install is enough.

And regarding permission issue, I also think that sudo should be only used when sidekiq_service_unit_user is system. But as I said, not directly related to this PR.

[jclusso]

@legendetm are you referring to the usage of sudo for sidekiq:install and sidekiq:uninstall? Yes the documentation could be updated to include that. I was wondering if maybe there should be a sidekiq:configure or sidekiq:reconfigure that really just runs uninstall/install under the hood to make it easier.

[legendetm]

@jclusso I meant we should mention in documentation that you need to run sidekiq:uninstall with before upgrading (before changing the sidekiq_processes count) and sidekiq:install after the changes are done. Something like you mentioned in your PR

Finally, in regards to issues around backward compatibility, you can simply run sidekiq:uninstall before upgrading and sidekiq:install after upgrading. This makes more sense than over complicating the code.

Other option would be sidekiq:configure logic, but this may be complicated when decreasing workers, then you need search all enabled services with or without arguments, but maybe there is already a command for this. But this would definitely make the changing the sidekiq_processes count easier. I'm not a maintainer, but I would prefer separate improvement PR for this when this is merged.

Regarding the sudo part, this is unrelated to this PR. Just in my case the deploy user has no sudo privileges and therefore uninstall fails, but this is tracked in separate issue and I can make PR latter if needed.

[jclusso]

@seuros where do we stand on getting this merged or are you integrating this into other changes?

[jclusso]

Hey @seuros, just checking in to see if you've had a chance to review this.

[seuros]

Can you try master branch or the prerelease?

[jclusso]

@seuros I just reviewed the v3.0.0.alpha.1 and it does not appear to be correctly using Systemd's multiprocess support. Did you get a chance to review everything I wrote above?

[jclusso]

@seuros checking in on this again since it's been quite some time.

[zedtux]

@seuros could you please help us to solve this issue impacting more and more people please? 🙏

[jclusso]

@seuros checking on this again...

[jclusso]

@seuros any chance you've gotten to review this?

[seuros]

I will fix this gem in this weekend, do you mind if i ping you for testing ?

[jclusso]

I will fix this gem in this weekend, do you mind if i ping you for testing ?

No problem - my availability this weekend may be limited, but I'll try and respond!

[jclusso]

@seuros is there any update on this?

[jclusso]

@seuros is there going to be an update on this?

[seuros]

I will look into it.