Merge lp:~adeuring/charms/precise/charmworld/charm-set-lock-timeout into lp:~juju-jitsu/charms/precise/charmworld/trunk

[Abel Deuring]

set the lease time for scripts locks from the juju config parameter execute-ingest-every

Get the lock lease time from the ingest cycle time

The scripts "queue" and "review" acquire locks to avoid concurrent
runs of these scripts on multiple instances.

The lease time for these locks can and should be set from the
"cycle time" of the scripts. Curtis suggested to keep the lock
for twice as long; I added one minute more to avoid possible
spurious errors that might occur due to clock skew between two
servers.

https://codereview.appspot.com/7422045/

[Abel Deuring (adeuring)]

Reviewers: mp+151056_code.launchpad.net,

Message:
Please take a look.

Description:
Get the lock lease time from the ingest cycle time

The scripts "queue" and "review" acquire locks to avoid concurrent
runs of these scripts on multiple instances.

The lease time for these locks can and should be set from the
"cycle time" of the scripts. Curtis suggested to keep the lock
for twice as long; I added one minute more to avoid possible
spurious errors that might occur due to clock skew between two
servers.

https://code.launchpad.net/~adeuring/charms/precise/charmworld/charm-set-lock-timeout/+merge/151056

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7422045/

Affected files:
   A [revision details]
   M scripts/write_config.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: scripts/write_config.py
=== modified file 'scripts/write_config.py'
--- scripts/write_config.py 2013-02-14 19:10:58 +0000
+++ scripts/write_config.py 2013-02-28 16:49:04 +0000
@@ -5,6 +5,7 @@
  import subprocess
  import sys

+from charmsupport.hookenv import config
  from charmsupport.hookenv import related_units
  from charmsupport.hookenv import relation_ids

@@ -18,6 +19,13 @@
  def write_new_config(mongo_url, default, override, target):
      new_config = read_config([default, override])
      new_config.set('app:main', 'mongo.url', mongo_url)
+ # script_lease_time specifies the time for which the scripts
+ # "queue" and "review" hold their locks. Set to twice the time
+ # interval execute-ingest-every, plus 1 minute slack to address
+ # a possible clock skew between two instances.
+ new_config.set(
+ 'app:main', 'script_lease_time',
+ config()['execute-ingest-every'] * 2 + 1)
      with open(target, 'w') as output:
          new_config.write(output)

[Aaron Bentley (abentley)]

execute-ingest-every is not necessarily the actual interval in the crontab, since values less than 5 or more than 60 are clamped. (See hooks/config-changed line 65 or so). Please do similar clamping (to ensure values less than five don't lead to premature timeouts).

review: Needs Fixing

[lp:~adeuring/charms/precise/charmworld/charm-set-lock-timeout]

48. By Abel Deuring on 2013-02-28

clamp the value of execute-ingest-every to the range 5..60

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'scripts/write_config.py'
--- scripts/write_config.py	2013-02-14 19:10:58 +0000
+++ scripts/write_config.py	2013-02-28 17:15:26 +0000
@@ -5,6 +5,7 @@
 import subprocess
 import sys
 
+from charmsupport.hookenv import config
 from charmsupport.hookenv import related_units
 from charmsupport.hookenv import relation_ids
 
@@ -18,6 +19,17 @@
 def write_new_config(mongo_url, default, override, target):
     new_config = read_config([default, override])
     new_config.set('app:main', 'mongo.url', mongo_url)
+    # script_lease_time specifies the time for which the scripts
+    # "queue" and "review" hold their locks. Set to twice the time
+    # interval execute-ingest-every, plus 1 minute slack to address
+    # a possible clock skew between two instances.
+    ingest_cycle = config()['execute-ingest-every']
+    if ingest_cycle < 5:
+        ingest_cycle = 5
+    if ingest_cycle > 60:
+        ingest_cycle = 60
+    new_config.set(
+        'app:main', 'script_lease_time', ingest_cycle)
     with open(target, 'w') as output:
         new_config.write(output)