Merge lp:~bac/charms/precise/charmworld/remove-queue-cronjob into lp:~juju-jitsu/charms/precise/charmworld/trunk

Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: 87
Merged at revision: 87
Proposed branch: lp:~bac/charms/precise/charmworld/remove-queue-cronjob
Merge into: lp:~juju-jitsu/charms/precise/charmworld/trunk
Diff against target: 155 lines (+52/-46)
5 files modified
README.md (+11/-6)
config.yaml (+8/-4)
hooks/config-changed (+23/-24)
scripts/write_config.py (+9/-10)
templates/crontab.tmpl (+1/-2)
To merge this branch: bzr merge lp:~bac/charms/precise/charmworld/remove-queue-cronjob
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+200310@code.launchpad.net

Description of the change

Change cron/lock behavior.

Rename configuration variable execute-ingest-interval to cron-interval.
Add a separate config script-lease-time.
Remove the cron entry for scripts/queue, as it is rolled into the worker now.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Please take a look.

Revision history for this message
Brad Crittenden (bac) wrote :

Please take a look.

Revision history for this message
Benji York (benji) wrote :

It was inherited from the earlier implementation, but using an interval of 30 minutes if the configured interval is over 60 minutes seems odd. Making 60 an upper bound would make more sense.

Other than that, this looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2013-04-01 19:38:54 +0000
3+++ README.md 2014-01-02 14:48:05 +0000
4@@ -67,16 +67,21 @@
5
6 juju set charmworld error-email=jrandom@example.com
7
8-## Ingest interval
9+## Cron interval
10
11-Adjust this to increase or decrease the time between ingest runs. Cron will
12-execute "scripts/ingest" which update and refresh the charm details, the
13-information in each queue, and all other inbound related data points. The
14-default for this is `15` mins. An interval of less than 5 would likely cause
15+Adjust this to determine how often periodic scripts are run by cron.
16+The default is `15` mins. An interval of less than 5 would likely cause
17 issues, so values of less than 5 are promoted to 5. A value of 0 will disable
18 the cron job.
19
20- juju set charmworld execute-ingest-every=15
21+ juju set charmworld cron-interval=15
22+
23+## Script lease time
24+
25+Sets the expectation for how cronjobs should execute. Scripts that run longer
26+will be forced to yield their locks.
27+
28+ juju set charmworld script-lease-time=15
29
30 ## Source location
31
32
33=== modified file 'config.yaml'
34--- config.yaml 2013-10-21 23:59:30 +0000
35+++ config.yaml 2014-01-02 14:48:05 +0000
36@@ -1,8 +1,12 @@
37 options:
38- execute-ingest-every:
39- type: int
40- default: 15
41- description: "The amount of time (in minutes) that the ingest tasks for charmworld are executed"
42+ cron-interval:
43+ type: int
44+ default: 15
45+ description: "The amount of time (in minutes) that periodic cron jobs should run."
46+ script-lease-time:
47+ type: int
48+ default: 15
49+ description: "The amount of time (in minutes) that scripts should hold locks."
50 worker-interval:
51 type: int
52 default: 1
53
54=== modified file 'hooks/config-changed'
55--- hooks/config-changed 2013-10-22 00:00:13 +0000
56+++ hooks/config-changed 2014-01-02 14:48:05 +0000
57@@ -120,30 +120,29 @@
58 exit 0
59 fi
60
61-interval=`config-get execute-ingest-every`
62-: ${interval:-0}
63-
64-if [ $interval -ne 0 ]; then
65- juju-log "Run the cron every $interval minutes"
66- if [ $interval -lt 5 ]; then
67- juju-log "Clamp to 5 minutes."
68- interval=5
69- elif [ $interval -gt 60 ]; then
70- juju-log "Eventually, we'll calculate hours. For now this is going to just run every half hour."
71- interval=30
72- fi
73- error_email=`config-get error-email`
74-
75- if [ -z "$error_email" ]; then
76- mailto=""
77- else
78- mailto="MAILTO=$error_email"
79- fi
80-
81- export mailto
82- export interval
83- cheetah fill --env -p templates/crontab.tmpl > /etc/cron.d/charmworld
84-fi
85+cron_interval=`config-get cron-interval`
86+: ${cron_interval:-0}
87+
88+juju-log "Run non-daily cron jobs every $cron_interval minutes"
89+if [ $cron_interval -lt 5 ]; then
90+ juju-log "Clamp to 5 minutes."
91+ cron_interval=5
92+elif [ $cron_interval -gt 60 ]; then
93+ juju-log "Eventually, we'll calculate hours. For now this is going to just run every half hour."
94+ cron_interval=30
95+fi
96+
97+error_email=`config-get error-email`
98+
99+if [ -z "$error_email" ]; then
100+ mailto=""
101+else
102+ mailto="MAILTO=$error_email"
103+fi
104+
105+export mailto
106+export cron_interval
107+cheetah fill --env -p templates/crontab.tmpl > /etc/cron.d/charmworld
108
109 if [ $could_ingest == 'false' ]; then
110 juju-log "Syncing database to index."
111
112=== modified file 'scripts/write_config.py'
113--- scripts/write_config.py 2013-09-20 12:39:25 +0000
114+++ scripts/write_config.py 2014-01-02 14:48:05 +0000
115@@ -31,17 +31,16 @@
116 handler_class, handler_args = error_handler
117 new_config.set('handler_exc_smtp', 'class', handler_class)
118 new_config.set('handler_exc_smtp', 'args', handler_args)
119- # script_lease_time specifies the time for which the scripts
120- # "queue" and "review" hold their locks. Set to twice the time
121- # interval execute-ingest-every, plus 1 minute slack to address
122- # a possible clock skew between two instances.
123- ingest_cycle = config()['execute-ingest-every']
124- if ingest_cycle < 5:
125- ingest_cycle = 5
126- if ingest_cycle > 60:
127- ingest_cycle = 60
128+ # script_lease_time specifies the time for which scripts
129+ # (e.g. askubuntu, github, and review, ) hold their locks before timing
130+ # out.
131+ script_lease_time = config()['script-lease-time']
132+ if script_lease_time < 5:
133+ script_lease_time = 5
134+ if script_lease_time > 60:
135+ script_lease_time = 60
136 new_config.set(
137- 'app:main', 'script_lease_time', ingest_cycle)
138+ 'app:main', 'script_lease_time', script_lease_time)
139 new_config.set(
140 'app:main', 'worker_interval', worker_interval)
141 new_config.set('app:main', 'es_replicas', es_replicas)
142
143=== modified file 'templates/crontab.tmpl'
144--- templates/crontab.tmpl 2013-10-10 18:24:57 +0000
145+++ templates/crontab.tmpl 2014-01-02 14:48:05 +0000
146@@ -2,8 +2,7 @@
147 PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
148 $mailto
149
150-*/$interval * * * * charmworld HOME=$charmworld_home LOGDIR=$log_dir INI=$CONFIG_FILE $webops_home/run-write-errors $log_dir/ingest-errors $project_dir/scripts/queue
151-*/$interval * * * * charmworld HOME=$charmworld_home LOGDIR=$log_dir INI=$CONFIG_FILE $webops_home/run-write-errors $log_dir/ingest-review-errors $project_dir/scripts/review
152+*/$cron_interval * * * * charmworld HOME=$charmworld_home LOGDIR=$log_dir INI=$CONFIG_FILE $webops_home/run-write-errors $log_dir/ingest-review-errors $project_dir/scripts/review
153 @daily charmworld HOME=$charmworld_home LOGDIR=$log_dir INI=$CONFIG_FILE $webops_home/run-write-errors $log_dir/askubuntu-errors $project_dir/scripts/askubuntu
154 @daily charmworld HOME=$charmworld_home LOGDIR=$log_dir INI=$CONFIG_FILE $webops_home/run-write-errors $log_dir/github-errors $project_dir/scripts/github
155 @daily charmworld HOME=$webops_home INI=$project_dir/production.ini $project_dir/bin/python $project_dir/charmworld/jobs/cstat.py

Subscribers

People subscribed via source and target branches

to all changes: