Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1065762 - use a simpler systemd service file
Summary: use a simpler systemd service file
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: spamassassin
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Warren Togami
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-16 18:18 UTC by Michael S.
Modified: 2014-03-01 01:36 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-27 21:59:51 UTC


Attachments (Terms of Use)
patch against git to simplify the serice file (deleted)
2014-02-16 18:18 UTC, Michael S.
no flags Details | Diff
Reverts previous patch partially (adds -d option to service file) (deleted)
2014-02-24 05:46 UTC, AMM
no flags Details | Diff

Description Michael S. 2014-02-16 18:18:06 UTC
Created attachment 863795 [details]
patch against git to simplify the serice file

Description of problem:
Current spamassassin file use Type=forking and a pidfile for tracking. While working, it is more complex than it need to be. Using Type=simple and not forking in the background permit to have a simpler file ( and faster to spawn, since we do not need to fork or anything ). 

Here is a patch against rawhide fixing this.

However, since '-d' is in /etc/sysconfig/spamassassin and in the configuration ( while it should be IMHO in the service file ), it would cause issues on upgrade.

Comment 1 Kevin Fenzi 2014-02-16 19:10:28 UTC
Applied in rawhide. Thanks. ;) 

Will go into 20/19 when/if 3.4.0 is stable enough...

Comment 2 AMM 2014-02-24 04:44:58 UTC
This patch floods /var/log/messages file.

The reason is, calling spamd without -d makes it write debug lines to stderr.

This stderr lines are caught by systemctl and redirected to syslog which in turn flood /var/log/messages.

Either patch should be reverted or stderr should be redirected to /dev/null.

Comment 3 Kevin Fenzi 2014-02-24 04:59:11 UTC
Whats in your /etc/sysconfig/spamassassin ? 

and is there a /etc/sysconfig/spamassassin.rpmnew 

and if you move that does it all work ok?

Comment 4 AMM 2014-02-24 05:22:44 UTC
No its default i.e. -c -m5 -H

I know -d option is removed.

As I said when you remove -d option to spamd, it enables inbuilt stderr logger.

i.e. it starts outputting stderr.

this output are redirected to syslog and hence appear in /var/log/messages.

Comment 5 AMM 2014-02-24 05:46:30 UTC
Created attachment 866869 [details]
Reverts previous patch partially (adds -d option to service file)

This reverts previous patch to service file.

It changes type back to forking and adds -d option. (which earlier was in sysconfig file)

No chnages are required to sysconfig file.

Comment 6 AMM 2014-02-24 05:52:12 UTC
We can not use 'simple' type in service file yet.

Because spamd without -d option enables stderr logging which floods /var/log/messages.

If we do not use -d in service file, then we need to redirect stderr to /dev/null. But then that adds extra CPU cycles to spamassin per logline and also duplicate logging. (same line appears in maillog and messages)

Solution is to propose upstream developers to make stderr logging optional (even if -d switch is not used). OR revert service file back to 'forking' type. (as per patch attached in my previous comment)

Comment 7 Kevin Fenzi 2014-02-24 15:24:20 UTC
What messages are you seeing in /var/log/messages? 

I can't duplicate the issue here... I don't get any there.

Comment 8 AMM 2014-02-24 16:05:45 UTC
Strange. One thing I forgot to mention earlier is I am on F16.

But I have used source RPM of rawhide without any change.

May be systemd of rawhide redirects stderr to /dev/null by default whereas systemd of F16 redirects to syslog.

Try running spamd directly instead of via systemctl as:
spamd -c -m5 -H

You will see that it prints debug information on stderr. (which atleast incase of F16 goes to syslog)

Here is the issue I reported on spamassassin mailing list. Which has some sample lines.

http://mail-archives.apache.org/mod_mbox/spamassassin-users/201402.mbox/%3C1393164434.96267.YahooMailNeo@web194604.mail.sg3.yahoo.com%3E

Comment 9 Kevin Fenzi 2014-02-27 21:59:51 UTC
Well, not sure whats going on here... I cannot duplicate this issue on rawhide or f20 at least. 

I can only assume it's something going on with systemd in f16.

Comment 10 AMM 2014-02-28 06:44:40 UTC
Umm, as I said, it may not be happening in F20, because systemd may be redirecting stderr to /dev/null.

You can see what actually happens by manually running spamd -c -m5 -H. It enables "stderr" logging and prints messages per e-mail.

This adds to extra CPU cycles (and stderr redirection per line) as for every e-mail, spamd calls additional function for stderr logging, in addition to syslog logging to maillog.

So we believe we saved one fork() by not running in daemonize (by removing -d option) mode but what actual happens is we add lakhs of lines of unwanted call to "stderr" logging function.

Comment 11 Kevin Fenzi 2014-02-28 22:52:45 UTC
Systemd doesn't send stderr to /dev/null. In recent/modern/supported systemd it sends that output to the journal. ;) 

Your systemd is before the journal existed. 

I agree it's less than ideal to have a copy in syslog and journal in this case. Will ponder on what best to do here...

Comment 12 AMM 2014-03-01 01:36:41 UTC
Ah ok, so finally you found that there is indeed double logging! My bad I was unaware about journal thing otherwise I could have pinpointed there right away instead of syslog.

But as I said its not just about double logging. It also slows down spamd a bit because it spends few extra miliseconds in debug calls (for every e-mail) which is not required.

Originally patch's purpose was to save a fork call, but it does not really save resources, in this case, but increases use of resources.

The idea of patch is correct but in my opinion, till upstream provides solution of disabling stderr logging, we should revert the previous patch partially.


Note You need to log in before you can comment on or make changes to this bug.