[Piuparts-devel] setting priority of sections
Dave Steele
dsteele at gmail.com
Sat Nov 26 13:00:27 UTC 2011
On Fri, Nov 25, 2011 at 10:23 AM, Holger Levsen <holger at layer-acht.org> wrote:
> Hi Andreas,
>
> On Mittwoch, 23. November 2011, Andreas Beckmann wrote:
...
>> * sections with a higher priority value will only send log files but not
>> request new work or run pending work as long as a section with lower
>> priority value has done some work
>
Here is a candidate patch that does this, if you replace the word
'section' with 'state'.
I've taken a look at implementing priority queues in the main loop of
piuparts-slave, for the purpose of flexibly rerunning old logs. The
code is at
git://github.com/davesteele/piuparts.git feature/flexible-retest
This version will test only 'waiting' packages if there are any.
Otherwise, it will process failed logs over a threshold age. After
that, 'successful' logs are rerun.
The code was tested over a rerun of the failed queue on sid.
There are a couple of things I noticed here:
1) There is much downloading/parsing of the Packages file, and
recalculating package states going on. Both are expensive.
2) The master process doesn't like package state changes it didn't
cause. There are opportunities for race conditions with multiple
slaves/master processes, which would need to be worked out.
'1' is an issue that section priority would have to deal with as well.
---------------------------------------------
This is what idle behavior looks like with this, running on a small-ish VM.
$ /usr/share/piuparts/piuparts-slave
07:35 Check-replace sid.tar.gz: age=381788 vs. max=2592000
07:35 -------------------------------------------
07:35 Running section sid, state waiting-to-be-tested
07:35 Connecting to localhost
07:36 Master didn't reserve anything (more) for us
07:36 Connection to master closed
07:36 -------------------------------------------
07:36 Running section sid, state failed-testing
07:36 Connecting to localhost
07:37 Master didn't reserve anything (more) for us
07:37 Connection to master closed
07:37 -------------------------------------------
07:37 Running section sid, state successfully-tested
07:37 Connecting to localhost
07:38 Master didn't reserve anything (more) for us
07:38 Connection to master closed
07:38 Nothing to do, sleeping for 300 seconds.
07:43 -------------------------------------------
07:43 Running section sid, state waiting-to-be-tested
...
---------------------------------------------
diff --git a/README.txt b/README.txt
index 4162c30..56f4f0e 100644
--- a/README.txt
+++ b/README.txt
@@ -267,7 +267,7 @@ the master responds with):
>> here
>> .
<< ok
->> reserve
+>> reserve waiting-to-be-tested
<< ok vorbisgain 2.3-4
----
@@ -282,7 +282,7 @@ The slave shall not speak until the master has spoken.
Commands and responses in this protocol:
----
-Command: reserve
+Command: reserve <state>
Success: ok <packagename> <packageversion>
Failure: error
----
@@ -292,6 +292,10 @@ packages to test. If the transaction fails, there
are no more
packages to test, and the slave should disconnect, wait some time
and try again.
+Valid states are currently "waiting-to-be-tested", "failed-testing",
+and "successfully-tested". The state defaults to "waiting-to-be-tested"
+if omitted.
+
----
Command: unreserve <packagename> <packageversion>
Success: ok
diff --git a/debian/changelog b/debian/changelog
index c739bc2..0bafaf0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -113,6 +113,14 @@ piuparts (0.41) unstable; urgency=low
moved to git, the URLs are the same, just s#piuparts#piatti#.
* Add entry about git to NEWS.
* Update Vcs-*-Headers in control to reflect move to git.
+ [Dave Steele]
+ * Add support for flexible package retesting
+ - piuparts-slave cycles through "waiting", "failed", and "successful" queue
+ if no packages are pending
+ - slave, master and PackagesDB reserve() call takes a 'state' argument
+ - Master API 'reserve' call takes an optional state, defaulting to
+ "waiting-to-be-tested"
+ - Config adds minimum age to consider retry, by state
-- Holger Levsen <holger at debian.org> Wed, 24 Aug 2011 13:47:07 +0200
diff --git a/piuparts-master.py b/piuparts-master.py
index 203a9c2..278abb9 100644
--- a/piuparts-master.py
+++ b/piuparts-master.py
@@ -54,6 +54,9 @@ class Config(piupartslib.conf.Config):
"packages-url": None,
"master-directory": ".",
"known_circular_depends": "",
+ "age-threshold-waiting-to-be-tested": 0,
+ "age-threshold-failed-testing": 7,
+ "age-threshold-successfully-tested": 30,
}, "")
@@ -111,7 +114,8 @@ class Master(Protocol):
"successfully-tested",
)
- def __init__(self, input, output, packages_file,
known_circular_depends="", section=None):
+ def __init__(self, input, output, packages_file, age_thresholds,
+ known_circular_depends="", section=None):
Protocol.__init__(self, input, output)
self._commands = {
"reserve": self._reserve,
@@ -132,6 +136,9 @@ class Master(Protocol):
logging.debug("section: "+section+" even more WTF should
this ever be logged with section lenny2squeeze...!")
except:
logging.debug("section: "+section+" a fine dose of WTF. I'm
looking forward to the day I understand this. This shouldnt happen and
only happens for the lenny2squeeze section, never for squeeze or sid.
")
+
+ self.age_thresholds = age_thresholds
+
self._writeline("hello")
def do_transaction(self):
@@ -156,8 +163,23 @@ class Master(Protocol):
logging.debug("%s : %s\n" % (st,pkg))
def _reserve(self, command, args):
- self._check_args(0, command, args)
- package = self._binary_db.reserve_package()
+ #self._check_args(0, command, args)
+ if len( args ) == 0:
+ state = "waiting-to-be-tested"
+ elif len( args ) == 1:
+ state = args[0]
+ else:
+ raise CommandSyntaxError( "Need 0 or 1 args: %s %s" %
+ ( command, " ".join(args)))
+
+ # todo - this is test code, to exercise the other states
+ # for people with wimpy VMs
+# if state == "waiting-to-be-tested":
+# self._short_response("error")
+# return
+
+ age_threshold = self.age_thresholds[ state ]
+ package = self._binary_db.reserve_package( state, age_threshold )
if package is None:
self._short_response("error")
else:
@@ -208,9 +230,17 @@ def main():
packages_file = piupartslib.open_packages_url(config["packages-url"])
known_circular_depends = config["known_circular_depends"]
- m = Master(sys.stdin, sys.stdout, packages_file,
known_circular_depends, section=section)
+ age_threshold = {}
+ for state in ( "waiting-to-be-tested", "failed-testing",
+ "successfully-tested" ):
+ age_threshold[ state ] = int(global_config[
"age-threshold-" + state ]) * 86400
+
+ m = Master(sys.stdin, sys.stdout, packages_file, age_threshold,
+ known_circular_depends, section=section )
+
while m.do_transaction():
pass
+
packages_file.close()
else:
print 'piuparts-master needs to be called with a valid
sectionname as argument, exiting...'
diff --git a/piuparts-slave.py b/piuparts-slave.py
old mode 100644
new mode 100755
index ef2971c..faff533
--- a/piuparts-slave.py
+++ b/piuparts-slave.py
@@ -180,8 +180,8 @@ class Slave:
if line != "ok\n":
raise MasterNotOK()
- def reserve(self):
- self._writeline("reserve")
+ def reserve(self, state):
+ self._writeline("reserve %s" % state)
line = self._readline()
words = line.split()
if words and words[0] == "ok":
@@ -267,9 +267,9 @@ class Section:
create_or_replace_chroot_tgz(self._config, tarball,
self._base_tgz_ctrl,
self._config["upgrade-test-distros"].split()[0])
- def run(self):
+ def run(self, state):
logging.info("-------------------------------------------")
- logging.info("Running section " + self._config.section)
+ logging.info("Running section %s, state %s" % (
self._config.section, state ) )
self._slave.connect_to_master(self._log_file)
oldcwd = os.getcwd()
@@ -284,11 +284,13 @@ class Section:
if not self._slave.get_reserved():
max_reserved = int(self._config["max-reserved"])
- while len(self._slave.get_reserved()) < max_reserved and
self._slave.reserve():
+ while len(self._slave.get_reserved()) < max_reserved and
self._slave.reserve( state ):
pass
self._slave.close()
+ test_count = len(self._slave.get_reserved())
+
if self._slave.get_reserved():
self._check_tarball()
packages_files = {}
@@ -324,6 +326,8 @@ class Section:
self._slave.forget_reserved(package_name, version)
os.chdir(oldcwd)
+ return( test_count )
+
def log_name(package, version):
return "%s_%s.log" % (package, version)
@@ -510,15 +514,16 @@ def main():
sections.append(section)
while True:
- for section in sections:
- section.run()
- idle = True
- for section_name in section_names:
- if os.listdir(os.path.join(global_config["master-directory"],section_name,"reserved")):
- idle = False
- if idle:
- logging.info("Nothing to do, sleeping for %s seconds." %
global_config["idle-sleep"])
- time.sleep(int(global_config["idle-sleep"]))
+ test_count = 0
+ for state in [ "waiting-to-be-tested", "failed-testing",
"successfully-tested" ]:
+ if test_count == 0:
+ for section in sections:
+ test_count += section.run( state )
+
+ if test_count == 0:
+ sleep_time = int(global_config["idle-sleep"])
+ logging.info("Nothing to do, sleeping for %d seconds." %
sleep_time )
+ time.sleep( sleep_time)
if __name__ == "__main__":
diff --git a/piupartslib/packagesdb.py b/piupartslib/packagesdb.py
index 1c9d27c..a026173 100644
--- a/piupartslib/packagesdb.py
+++ b/piupartslib/packagesdb.py
@@ -29,6 +29,7 @@ import os
import random
import tempfile
import UserDict
+import time
from piupartslib.dependencyparser import DependencyParser
@@ -181,6 +182,7 @@ class LogDB:
return "%s_%s.log" % (package, version)
def log_exists(self, package, subdirs):
+ # to do - use get_log_path
log_name = self._log_name(package["Package"], package["Version"])
for subdir in subdirs:
if self.exists(os.path.join(subdir, log_name)):
@@ -230,6 +232,16 @@ class LogDB:
if self.exists(full_name):
self.remove_file(full_name)
+ def get_log_path(self, subdir, package, version ):
+ return( os.path.join( subdir, self._log_name( package, version ) ) )
+
+
+ def get_log_age(self, subdir, package, version ):
+ c_time = os.path.getctime( self.get_log_path( subdir,
package, version ) )
+ age = time.time() - c_time
+
+ return( int( age ) )
+
class PackagesDB:
@@ -273,6 +285,12 @@ class PackagesDB:
self.set_subdirs(ok="pass", fail="fail", evil="untestable",
reserved="reserved", morefail=["bugged"])
+ self.state_path = {
+ "waiting-to-be-tested": None,
+ "failed-testing": self._fail,
+ "successfully-tested": self._ok
+ }
+
def set_subdirs(self, ok=None, fail=None, evil=None,
reserved=None, morefail=None):
# Prefix all the subdirs with the prefix
if self.prefix:
@@ -547,15 +565,31 @@ class PackagesDB:
else:
return "unknown"
- def _find_packages_ready_for_testing(self):
- return self.get_packages_in_state("waiting-to-be-tested")
+ def _find_packages_ready_for_testing(self, state, time_threshold ):
+ logdir = self.state_path[ state ]
+
+ return [ p for p in self.get_packages_in_state( state )
+ if self._logdb.log_exists( p, [self.state_path[ state ]] )
+ and self._logdb.get_log_age(logdir, p["Package"],
p["Version"])
+ > time_threshold ]
+
+ def reserve_package(self, state, time_threshold ):
+ plist = self._find_packages_ready_for_testing( state, time_threshold )
- def reserve_package(self):
- plist = self._find_packages_ready_for_testing()
random.shuffle(plist)
for p in plist:
if self._logdb.create(self._reserved, p["Package"],
p["Version"], ""):
+
+ logdir = self.state_path[ state ]
+ if logdir and self._logdb.log_exists( p, [ logdir ] ):
+
+ self._logdb.remove( logdir, p["Package"], p["Version"])
+
+ self._in_state[state] = [ s for s in self._in_state[state]
+ if s != p["Package"] ]
+ self._in_state["waiting-to-be-tested"].append( p["Package"] )
+
return p
return None
More information about the Piuparts-devel
mailing list