-
Notifications
You must be signed in to change notification settings - Fork 657
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
Don't write a pid file when in socket activation mode #2143
Comments
very good suggestion, thx! Would it be appropriate to do not write (nor check) pid files if Comments? @davidelang ? |
Sounds ok to me. The altenative would be to check for getenv("LISTEN_PID") resp. if nFds > 0 |
@mbiebl What would you find to be more appropriate. I think that "fixing" |
I don't have a strong preference. Both would be ok to me. |
thx -- I'll keep it open a bit for further comments. Will change it in 8.32 timeframe for sure. |
I'd say it's better to keep current behaviour for backward compatibility but provide specific cmdline switch to disable pidfile creation ( Just remember there are old logrotate scripts which are not aware of systemd still (check /etc/logrotate.d/syslog in CentOS RPM for example). |
I assume those old logrotate scripts check for the existence of a pid file and would be broken as well once the .service file start using |
I thought that is a good point. Just curios: how will logrotate work when no pidfile is given? |
I would hope that the /etc/logrotate.d/syslog script is provided by the rsyslog package and updated along with the change to not write a pid file. Ideally it wouldn't check for the pid file at all |
ok, let me re-phrase my question: how can the script learn rsyslog's pid from systemd? Sorry for the basic question, but you probably know out of your head and I need to search around ;-) |
It's easy to fix rsyslog rpm to call systemctl instead of kill |
Why would the script need to know that in the first place? |
consider this answered ;-) |
And I'd say @jay7x actually provided a good reason for a separate option, especially as we weren't strong in favor of one of those... |
Are there really many scripts which need to know the rsyslog pid? the logrotate one is the only one that comes to mind here and in case of Debian, the logrotate script is provided by the rsyslog package itself, so would obviously be updated as well (in Debian the logrotate script does not use the pid file anymore, so wouldn't be affected anyway)
well, I don't see how this would magically fix those scripts once the rsyslog.service file uses that new option? |
BTW do we still support CentOS 6 packaging? There is no systemd. I guess that's why we still have that scary |
@jay7x To the best of my knowledge, it doesn't use |
@mbiebl Actually you're right.. but if we will change default behaviour then we may break every installation. |
you win ;-) |
which package does provide /etc/logrotate.d/syslog in CentOS? |
Just to be clean. I'm ok with any decision. Just raised my concerns here :) |
So, there is a problem indeed if there exist scripts which need to know the pid of the rsyslog process and use the pid file for that. What I'm trying to get a better overview is, if there might actually exist such scripts aside from the logrotate case. And assuming the logrotate script is shipped by the rsyslog package itself, I don't see a problem in this specific case. |
IMHO I think this can actually exist, for example in custom log rotation scripts/packages (besides logrotate). I am sure there are some solutions in e.g. Java. I barely remember having heared about that once or twice during the past couple of years. I don't think, however, that this is a reason NOT to implement the proposed change. It's more a good visible warning in ChangeLog IMHO. |
Just found it finally here: Installed by rsyslog package. v8 spec is here: |
Well.. we may not create pid file when 1) there is '-n' cmdline option AND 2) no '-i' cmdline option is specified. But we must fix supplied logrotate rule then. And man pages BTW. |
Sorry for the late reply
I think there should ba a way to not write a pid file, but I don't like changing
the behavior and not writing it automatically. There are too many cases where -n
is used for testing, and without a PID file, log rotation scripts will get
confused.
|
On Fri, 1 Dec 2017, Michael Biebl wrote:
well, I don't see how this would magically fix those scripts once the rsyslog.service file uses that new option?
it won't, but that's better than silently changing behavior just because the -n
flag is used.
|
On Fri, 1 Dec 2017, Michael Biebl wrote:
So, there is a problem indeed if there exists scripts which need to know the
pid of the rsyslog process and use the pid file for that. What I'm trying to
getter overview is, if there might actually exist such scripts aside from the
logrotate case. And assuming the logrotate script is shipped by the rsyslog
package itself, I don't see a problem in this specific case.
logrotate is not the only way to manage log rotation, so just fixing logrotate
doesn't mean that we won't break other people's scripts.
|
@davidelang so, assuming we add this new switch, and once it's available we use it in the systemd rsyslog.service file, what exactly would be different to having rsyslog not write a pid file automatically when being socket activated? |
On Fri, 8 Dec 2017, Michael Biebl wrote:
@davidelang so, assuming we add this new switch, and once it's available we
use it in the systemd rsyslog.service file, what exactly would be different to
having rsyslog not write a pid file automatically when being socket activated?
Other distros that don't change the flags won't get different behavior.
Not everything is systemd
|
I don't understand your reply. The behaviour in the non-socket activated case would not change. |
I'm catching up over a bunch of posts, and the early part of this discussion was
talking about not writing the PID file if the -n flag was provided. That is the
main thing I was objecting to (it changes behavior on people)
I understood you to be asking what the difference was between not writing a pid
file if an explicit flag is given that's made part of the standard systemd
config vs changing the behavior of an existing flag.
If someone add a flag to explicitly disable the PID file, and it doesn't work on
their system, that's their problem.
I think it's probably a mistake to not write the PID file (it's very useful for
log rotation scripts, and not everything is logrotate), but that's up to the
distro to decide :-)
|
Thanks for the clarification, @davidelang b/ do not write a pid file if rsyslog has been socket activated by systemd, i.e. if nFds > 0. This would require no changes to rsyslog.service |
On Fri, 8 Dec 2017, Michael Biebl wrote:
Thanks for the clarification, @davidelang
So if you got you right, your main objection is to change the behaviour of -n. I guess I can sympathize with that.
So I guess we have two options left then:
a/ do not write a pidfile if explicitly asked via a command line switch, like --no-pidfile.
The upstream rsyslog.service file would be updated to pass --no-pidfile by default I assume.
This would be my choice.
b/ do not write a pid file if rsyslog has been socket activated by systemd, i.e. if nFds > 0
I hate heuristics like this, are there other ways that this condition could be
triggered?
David Lang
|
Command line option -iNONE provides this capability. This utilizes the pre-existing -i option, but uses the special name "NONE" to turn of the pid file check feature. Turning off is useful for systems where this no longer is needed (e.g. systemd based). closes rsyslog#2143
Command line option -iNONE provides this capability. This utilizes the pre-existing -i option, but uses the special name "NONE" to turn of the pid file check feature. Turning off is useful for systems where this no longer is needed (e.g. systemd based). closes rsyslog#2143
I have extended the |
Still digging into it, but it looks like installing 8.33.0 on a stock Ubuntu 16.04 system results in I reported this in rsyslog/rsyslog-pkg-ubuntu#74 |
Sorry, I didn't qualify my remark: I meant that after installing 8.33.0 from the PPA that I observed that behavior. |
Confirmed that CentOS 7 has the same problem (not the same file mind you): the logrotate configuration expects to reference a PID file, but the rsyslog systemd unit file was configured to start rsyslog with the option to skip creating a PID file. Created a separate issue for that: rsyslog/rsyslog-pkg-rhel-centos#42 |
One workaround is to override the startup options using a systemd unit file drop-in. That has worked well enough in the past for us. |
That doesn't work in this case.
|
Nevermind: https://askubuntu.com/questions/659267/how-do-i-override-or-configure-systemd-services This seems to work:
|
At least for the Debian package, that is not true:
|
See, no pid file needed when running under systemd |
A pid file may not strictly be needed, but the files provided by the related packages on Ubuntu 16.04 and CentOS 7 are setup to use a pid.
From the /etc/logrotate.d/syslog file on a CentOS 7 system:
No pid is created for the rsyslog 8.33.0 package for CentOS 7 ( @mbiebl My guess would be that at some point you updated the Debian packages to use the settings you specified in a prior post, but it doesn't appear that the same settings are used in the "upstream" packages. |
Then fix the Ubuntu packages to not use a pid file in the logrotate config. Easy, right? |
I mean, that's part of maintaing packages. If the adiscon provided packages do not handle this correctly, then they should be fixed indeed. |
That would be logical, yes, but the net effect is that the 8.33.0 packages (as-is) break existing configurations when deployed. The related conf files provided by those packages will need to be updated to no longer look for the pid file. Until that happens, I will use a systemd drop-in to restore the old behavior. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
When rsyslog is used under systemd, it uses socket activation and the sd_notify API to signal service readiness. In that case, writing (and cleaning up) a pid file is not necessary. It's actually harmful if you want to lock down rsyslog.service with the systemd builtin mechanisms, as you need one more writable directory.
Please consider not writing a pid file when using socket activation mode or provide a command line switch which allows to turn off the pid file creation and can be used in rsyslog.service.
Thanks for considering
The text was updated successfully, but these errors were encountered: