awstats.pl cronjob spawns too many instances resulting in very high load average

[Steven Wagner]

Binary package hint: awstats

cronjob located in /etc/cron.d/awstats
0,10,20,30,40,50 * * * * www-data /usr/lib/cgi-bin/awstats.pl -config=awstats -update >/dev/null

If not completing within the 10 minute interval, multiple instances will be spawned until the machine comes to a halt. A check should be in place to prohibit additional instances from running if the prior instances is still running.

[Arne Rusek (zonk-matfyz)]

Package: awstats
Version: 6.5+dfsg-1
Followup-For: Bug #224506

Dear all.

I wrote myself a awstats cron wrapper which should solve at least three
bugreports on awstats :-)

It does:
  * locking: no more than one instance -- one configuration -- will run
 * config checking: check required config file (and included confs)
 * check that required logfile is available
 * should work out of the box when run from /etc/cron.d/awstats
 * it also makes awstats more nice :-)

If it's of any use, please include it into the awstats package and run
from cron. Questions, comments, suggestions and complains are welcomed
as always.

All the best,
Arne

-- System Information:
Debian Release: 4.0
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable')
Architecture: i386 (i686)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.17-maxipes-fik
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)

[Jonas Smedegaard (dr)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Arne Rusek wrote:

> I wrote myself a awstats cron wrapper which should solve at least three
> bugreports on awstats :-)
>
> It does:
> * locking: no more than one instance -- one configuration -- will run
> * config checking: check required config file (and included confs)
> * check that required logfile is available
> * should work out of the box when run from /etc/cron.d/awstats
> * it also makes awstats more nice :-)
>
> If it's of any use, please include it into the awstats package and run
> from cron. Questions, comments, suggestions and complains are welcomed
> as always.

Excellent work.

I do actually have some comments:

  * Don't ever use a static filename below /tmp.

Use mktemp instead!

  * Don't allow variables being overridded in ENV.

Instead first declare the variables as plain static values, and then
source some config file. Have a look at /etc/init.d/skeleton for an example.

  * Always quote strings containing variables!

  * Don't invoke script itself without safety belt against loops

It might not work within awstats either, but it seems to me that your
little script will loop forever if an include file includes itself.

  * Avoid bashisms if possiple

I don't use the "local" keyword myself, so don't know if it is a bashism.

You can drop the "function" keyword with no ill effect.

Change combined tests from "[ A -a B ]" to "[ A ] && [ B ]".

Test if the script (with bashisms removed) works with dash, and if it
does then change the hashbang from #!/bin/bash to #!/bin/sh

  * Are you sure about your quotes within quotes within quotes?

They seem risky to me.

  * Why invent the wheel?

Why use custom lock routine and not either lockfile (from the procmail
package) or preferrably the tools from thwe package liblockfile-progs?

  * Parsing perl code using shell?

What about ignoring stuff below __EOF__ (or whatever the magic perl
string is)?

Are you sure there's no other surprises when you try interpreting perl
code using shell code?

I mean - you are proposing this as a general tool, so it should work for
all, not just your own coding style.

Hope I am not too hard on you :-)

Charles Fry wrote:
> Also, could you suggest where the script should be installed? /usr/sbin?
>
> And named? awstats-run?

(Please let's work in the open, Charles!)

I agree with /usr/sbin as location.

I suggest calling it update-awstats, to follow the trend of other
update-* scripts.

But I feel that it need more work to be usable. Seems too risky to me to
apply by default as is.

 - Jonas

- --
* Jonas Smedegaard - idealist og Internet-arkitekt
* Tlf.: [phone] Website: http://dr.jones.dk/

 - Enden er nær: http://www.shibumi.org/eoti.htm
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFx1pDn7DbMsAkQLgRAtIxAJ99b3m8COdxSVbyvEQ3ySQF26PhfACeNFwG
M76x3a0J5kDSwyMX6dPsSuA=
=P4kT
-----END PGP SIGNATURE-----

[Arne Rusek (arne-rusek)]

Mon, Feb 05, 2007 at 05:24:36PM +0100, Jonas Smedegaard napsal:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Arne Rusek wrote:
>
> > I wrote myself a awstats cron wrapper which should solve at least three
> > bugreports on awstats :-)
> >
> > It does:
> > * locking: no more than one instance -- one configuration -- will run
> > * config checking: check required config file (and included confs)
> > * check that required logfile is available
> > * should work out of the box when run from /etc/cron.d/awstats
> > * it also makes awstats more nice :-)
> >
> > If it's of any use, please include it into the awstats package and run
> > from cron. Questions, comments, suggestions and complains are welcomed
> > as always.
>
>
> Hope I am not too hard on you :-)
No, I'm not offended at all. I'm working on improovments :) Anyway the
config file testing section is useless, I thought it's needed but amavis
handles the condition finely.

I'm just too busy right now... I'll email it later

> Charles Fry wrote:
> > Also, could you suggest where the script should be installed? /usr/sbin?
> >
> > And named? awstats-run?
>
> (Please let's work in the open, Charles!)
>
> I agree with /usr/sbin as location.
>
> I suggest calling it update-awstats, to follow the trend of other
> update-* scripts.
>
>
>
> But I feel that it need more work to be usable. Seems too risky to me to
> apply by default as is.
>
>
> - Jonas
>
> - --
> * Jonas Smedegaard - idealist og Internet-arkitekt
> * Tlf.: [phone] Website: http://dr.jones.dk/
>
> - Enden er nær: http://www.shibumi.org/eoti.htm
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFFx1pDn7DbMsAkQLgRAtIxAJ99b3m8COdxSVbyvEQ3ySQF26PhfACeNFwG
> M76x3a0J5kDSwyMX6dPsSuA=
> =P4kT
> -----END PGP SIGNATURE-----
>

[Steven Wagner (stevenwagner)]

Binary package hint: awstats

cronjob located in /etc/cron.d/awstats
0,10,20,30,40,50 * * * * www-data /usr/lib/cgi-bin/awstats.pl -config=awstats -update >/dev/null

If not completing within the 10 minute interval, multiple instances will be spawned until the machine comes to a halt. A check should be in place to prohibit additional instances from running if the prior instances is still running.

[Sergey B Kirpichev (skirpichev)]

Why not use EnableLockForUpdate option (works too for multiple site
configurations)? This option is in upstream from 5.0 release and
seems to be a simplest solution of the original problem.

Documentation:
http://awstats.sourceforge.net/docs/awstats_config.html#EnableLockForUpdate

If other developers would agree, I'll set the severity of this bug to
wishlist and retitle one accordingly (awstats cron wrapper).

[Sergey B Kirpichev (sk-darkstar)]

retitle 224506 awstats cron wrapper
severity 224506 wishlist
thanks

[papukaija (papukaija)]

Thank you for taking the time to report this bug and helping to make awstats better. You reported this bug a while ago and there hasn't been any activity in it recently. We were wondering if this is still an issue for you. Can you try with the latest Ubuntu release? Thanks in advance.

[papukaija (papukaija)]

We'd like to figure out what's causing this bug for you, but we haven't heard back from you in a while. Could you please provide the requested information? Thanks!

[Andreas Olsson (andol)]

I can confirm this potential issue even in Karmic and awstats 6.9~dfsg-1ubuntu1.

@papukaija: I would say the original reporter already have provided the relevant set of information. This being a rather general and easy to reproduce, the helpful thing would have been to self check out the status in Karmic.

[Andreas Olsson (andol)]

@Steven: The option EnableLockForUpdate might be a solution for you?

[papukaija (papukaija)]

@Andreas: Sorry, but I'm not using Karmic at the moment.

[papukaija (papukaija)]

From upstream report:
"In README.Debian suggestion was added to enable
EnableLockForUpdate=1. This option is in upstream from 5.0 release
and
seems to be a simplest solution of the original problem. But it has
security pitfalls, so it can't be enabled by default."