FS#18292 - [dcron] Should create ID Flag when not given to be compatible to his old behaviour

Attached to Project: Community Packages
Opened by Michael Trunner (trunneml) - Thursday, 11 February 2010, 13:41 GMT
Last edited by Evangelos Foutras (foutrelis) - Thursday, 17 January 2013, 01:19 GMT
Task Type Bug Report
Category Upstream Bugs
Status Closed
Assigned To Pierre Schmitz (Pierre)
Sergej Pupykin (sergej)
Architecture All
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:
Since version 4 dcron needs an ID Flag for @hourly, @daily, ... . Because of this nearly all of my cronjobs stopped working.
I think dcron should create a hash value of the command and use this as ID, if the cronjob has no ID flag. This way it is compatible to his old behaviour.

This is a problem for software that creates normal @hourly crontabs entries and don't add an ID flag, to be not dcron specific. (I know that ID=blabla, is handeld on other cronds as an env variable :-) )

Additional info:
* dcron 4.4-1
* dcron homepage tolds me to report bugs here
* Because most backup software use such crontabs entries I think the Severity is high.

Steps to reproduce:
* Add a cronjob with option @hourly and do not add ID=<something> in the command field
* In syslog you will find some error messages for example like this:
uucp.log:Feb 10 19:40:01 hostname crond[2153]: failed parsing crontab for user trunneml: writing timestamp requires job nice -n 19 /usr/bin/backintime --backup-job >/dev/null 2>&1 to be named

This task depends upon

Closed by  Evangelos Foutras (foutrelis)
Thursday, 17 January 2013, 01:19 GMT
Reason for closing:  Upstream
Additional comments about closing:  As per the last comment, please report bugs to the upstream Github repo.
Comment by Allan McRae (Allan) - Thursday, 11 February 2010, 13:59 GMT
Added developer to the notifications list.
Comment by Jim Pryor (Profjim) - Thursday, 11 February 2010, 14:38 GMT
There's no question here of compatibility with the old behavior *of dcron*. Its old behavior was to have no parsing of @daily... fields at all. This was the easiest way to make those fields available within the existing codebase.

If there were to be a default ID, I agree a hash of the command looks like the best way to go. I'll chew on whether there's some way to work that in, without architectural changes or adding dependencies. But I'm afraid it won't happen soon. (Of course I'll consider patches.)

In the meantime, you have to (1) get in the habit of giving an ID tag when you use @daily... fields, (2) don't use @daily fields, or (3) use a heavier-weight cron daemon.

Comment by Michael Trunner (trunneml) - Thursday, 11 February 2010, 17:47 GMT
Here is a small patch. Only a small problem is that cl_JobName is null and is so outputed in Debug.
Comment by Jim Pryor (Profjim) - Thursday, 11 February 2010, 18:32 GMT
Thanks. I see you're ignoring collisions. So we just let the crontab parsing fail if there are such, same as if the user had specified duplicate IDs. I like that, it's practical and probably good enough.

I'll work this into the git version soon.
Comment by Michael Trunner (trunneml) - Thursday, 11 February 2010, 19:01 GMT
It tried to get a good hash function:
http://en.literateprograms.org/Hash_function_comparison_%28C,_sh%29#chunk%20def:hashfuncs

I think sdbm is good enough. Normally a user has not that count of crons that a collision can really happen :-)
But I have a better version in mind. May be to night I can create a better patch.
Comment by Jim Pryor (Profjim) - Thursday, 11 February 2010, 19:46 GMT
I think sdbm will be good enough too.
Comment by Michael Trunner (trunneml) - Thursday, 11 February 2010, 21:45 GMT
This version look better and uses the whole crontab line for the hash not only the command part.
That fixes some hash-collision, when the same command is called on different lines.
And saves the hash in the line struct, for a later use.

Hope it is okay, and I will find it in the next release ;-)
Comment by Jim Pryor (Profjim) - Thursday, 11 February 2010, 23:03 GMT
I'm reluctant to use the whole crontab line for the hash; that means that any change no matter how trivial (e.g. whitespace) would wreck one's timestamps. (And would mean more hashes generated overall in practice, which would make it more likely that we might mistakenly collide with an abandoned timestamp file.)

But thanks for the patches, I'll work them in somehow. There's a big queue of changes waiting to be pushed to the public git repo.
Comment by Michael Trunner (trunneml) - Thursday, 11 February 2010, 23:13 GMT
I believe you will get more collision with crontab lines that have the same cmd, like backup crons as with old timestamp file.
At least such a collision with an old file makes no problem. The only thing that can happen, is that its first start is delayed for max one day. Thats all.
The other version will make problems every call, again and again and again.

And on new TeraByte harddirves some small files more or less makes no difference.
So the better version is to use the complete line. I started this version because a friend of mine on the university told me that my first version will make problems.

By the way you can add an cronjob that kills all files that are older then 2 weeks in the timestamp spool. :-)
Comment by Jim Pryor (Profjim) - Thursday, 11 February 2010, 23:38 GMT
1. Both these points are fair.
2. I wasn't worried about the number of timestamp files, but about collisions with them. But as you observed, the downside of a timestamp collision is at worst a delay of one full period---which may be longer than a day, e.g., if it's a @weekly or @monthly job, but it's still at worst a delay of one period.
That's a minor pain, but you're right the collisions with crontab lines having the same command may be more frequent---and more painful, since crond will just refuse to handle any lines whose hash has already been used.
3. There already is a sample cronjob that kills old timestamp files. But since there are @monthly cronjobs (and longer, if anyone has use for them), running this every 2 weeks is too frequent. The example I think runs every 90 days. IIRC, this is not installed by the Arch package. (Maybe I even made it after the last Arch release, I forget now.)
Comment by Michael Trunner (trunneml) - Friday, 12 February 2010, 10:48 GMT
Yes you are right there are monthly commands to. Ups -- my fault, again. :-)
So the clean up cronjob should run every hour, day or month and delete all timestamp files, that were much older then the longest supported period.
Comment by Paul Mattal (paul) - Monday, 22 February 2010, 02:06 GMT
Jim will be handling this in the next major release of dcron. We'll keep this open until then.
Comment by Paul Mattal (paul) - Thursday, 10 June 2010, 15:34 GMT
Hey Jim, any progress on this one?
Comment by roko (roko) - Monday, 20 June 2011, 22:50 GMT
Hi Jim, a year has passed and you haven't answered yet... So how is it going?
Comment by Alexander F. Rødseth (xyproto) - Monday, 05 November 2012, 00:51 GMT
This is currently our oldest [community] bug, with no new release of dcron since the 1st of May 2011.
Please consider moving this package to unsupported, if it is the case that dcron is not supported anymore.
Comment by Alexander F. Rødseth (xyproto) - Sunday, 11 November 2012, 21:09 GMT
Assigning to the maintainers of the packages that depend on this one.
Comment by Jim Pryor (Profjim) - Monday, 12 November 2012, 00:15 GMT
Hi, I'm the upstream developer of dcron. I aim to make a fresh release sometime in the next couple months---as soon as I get a few free minutes---and will implement this then. In the meantime, there's no reason you guys need to keep track of it. It's a feature request not a bug; I have a github repository at <https://github.com/dubiousjim/dcron> and future bug reports / feature requests can be entered there; and I have noted and will respond all the existing reports on the Arch bugtracker.

Loading...