Example cron action_trigger_runner.pl granularity issues

Bug #1468096 reported by Josh Stompro
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
New
Undecided
Unassigned

Bug Description

Hello, I think that the current example crontab file included with evergreen is incorrect when it comes to running pending action trigger events, and it doesn't try to process hooks for events without a granularity set at all. These issues add a few barriers to new users/testers that don't know that the example config won't work.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=blob;f=Open-ILS/examples/crontab.example;hb=HEAD
The current example has the following structure. (with extra stuff cleared out)

# Runs all pending A/T events every half hour
*/30 * * * * . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --run-pending

# Passive A/T event generation.
# Note: push these back to 3am so they will run after the fine generator and spread out the start minute to reduce dogpiling
0 * * * * . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity hourly
5 3 * * * . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity daily
10 3 * * 1-5 . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity weekdays
15 3 * * 0 . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity weekly
20 3 1 * * . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity monthly
25 3 1 1 * . ~/.bashrc && $EG_BIN_DIR/action_trigger_runner.pl --process-hooks --granularity yearly

So there are a bunch of runs for the various different granularity settings that just process hooks and generate pending events.. and one --run-pending run every half an hour to process all the pending events.

But the query that the --run-pending event uses to fire pending events looks for granularity:null. So it won't run pending events that have a granularity set.

[2015-06-23 13:55:52] open-ils.trigger [INFO:20849:CStoreEditor.pm:114:] editor[1|0] request en-US open-ils.cstore.direct.action_trigger.event.id_list [{"+atevdef":{"granularity":null},"run_time":{"<":"now"},"state":"pending"},{"order_by":{"atev":["run_time","add_time"]},"join":"atevdef"}]

Also, there is no action_trigger_runner.pl instance to process hooks that don't have a granularity set, so passive events that have no granularity will never get processed.

I've been fixing these issues by adding a --run-pending argument to each of the Passive A/T event generation lines, so the events are run right after they are processed. I'm wondering if there is a drawback to this method though. I assume there was a good reason to setup the example like it is, with only one run-pending every half an hour to handle all of the pending events at once.

Also, should a granularity be required for passive event definitions to be processed? Or should there be another crontab entry to process those ever xx minutes? I'll create a branch with changes that work for me.
Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Also, I think that the "~/.bashrc &&" can be removed also from the start of each action trigger line, since in my default debian .bashrc , the first thing it does is check to see if it is being run interactively, and exit if it is not.

# ~/.bashrc: executed by bash(1) for non-login shells.
# see /usr/share/doc/bash/examples/startup-files (in the package bash-doc)
# for examples

# If not running interactively, don't do anything
case $- in
    *i*) ;;
      *) return;;
esac

Josh

Revision history for this message
Thomas Berezansky (tsbere) wrote :

Actually, on all of our production systems I have had to put the opensrf PATH modification line *above* that "not running interactively" line in the .bashrc - Though I suppose actually setting PATH in the example crontab could work as well.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

user/stompro/lp1468096_example_crontab_update
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1468096_example_crontab_update

Here is a branch with my attempt to clean up the example crontab somewhat. If anyone has suggestions of other items that should have an entry there, please let me know and I'll add it.

Josh

Revision history for this message
Thomas Berezansky (tsbere) wrote :

Due to lack of /openils/bin being in the path I get this:

Can't exec "eg_config": No such file or directory at /openils/bin/sitemap_generator line 223.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

My preference would be to just add the path to the cron file so it doesn't depend on behavior that may be different between distro's / an undocumented adjustment to the opensrf install procedure.

I think that setting the --config-file= option to sitemap_generator will bypass that issue, or sitemap_generator should be modified to not depend on the PATH being set correctly.

It uses the following when no config file is chosen. So this depends on the $PATH but can handle custom CONF folders at runtime by default.
if (!$config_file) {
    my @temp = `eg_config --sysconfdir`;
    chomp $temp[0];
    $sysconfdir = $temp[0];
    $config_file = File::Spec->catfile($sysconfdir, "opensrf.xml");
}

vs the action_trigger_runner.pl.in that sets the default by just setting an absolute default path for the config file at build time. Which doesn't depend on the PATH and can handle being build with a custom CONF location.
my $opt_osrf_config = '@sysconfdir@/opensrf_core.xml';

I prefer the method that action_trigger_runner.pl.in uses since it removes a dependency on the PATH being set correctly. Maybe sitemap_generator can be modified to use the same method. Consistency would be good.

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I suppose that crontab.example could be run through the build process also to set custom paths correctly, for down the road when evergreen get packaged and needs to comply with the LSB file layouts.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I pushed a commit that should fix the sitemap_generator issue. Also added a few more examples for other scripts that someone may want to run and updated the env variable. I didn't add the PATH because I don't think it is needed and shouldn't be relied upon.

Let me know people disagree though.. if we do set the PATH = /openils/bin then all the commands can remove the $EG_BIN_DIR/ prefix and be a little clearer.

Josh

tags: added: actiontrigger
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.