[Demi-commits] r68 - / lib

[John Morrissey]

Author: jwm-guest
Date: 2006-01-30 18:05:18 +0000 (Mon, 30 Jan 2006)
New Revision: 68

Modified:
   TODO
   lib/agent.py
Log:
- make demi.agent._writeStatus() atomic; we'll write to
  commandName-results-temp and rename it when finished
- improve agent lock handling by using O_EXCL when performing
  locking


Modified: TODO
===================================================================
--- TODO	2006-01-27 20:27:51 UTC (rev 67)
+++ TODO	2006-01-30 18:05:18 UTC (rev 68)
@@ -1,4 +1,3 @@
-demi.agent _writeStatus() should be atomic; write to a temp file and rename()?
 http://www.dscpl.com.au/projects/vampire/
 autodetect demi web root
   req.uri, req.filename, and/or req.path_info seem useful

Modified: lib/agent.py
===================================================================
--- lib/agent.py	2006-01-27 20:27:51 UTC (rev 67)
+++ lib/agent.py	2006-01-30 18:05:18 UTC (rev 68)
@@ -29,7 +29,7 @@
 def processCommands():
 	"""Process commands queued on the current machine."""
 
-	if not _lockCommandDir():
+	if not _lockAgentDir():
 		return
 
 	files = os.listdir(AGENT_ROOT + "/commands/")
@@ -38,7 +38,13 @@
 
 	try:
 		for filename in files:
-			# Skip this command if it's already been executed.
+			# Skip temporary output files.
+			# FIXME: should we clean up after ourselves by removing this
+			# file? Only remove it after an interval (n days/weeks)?
+			if filename[-5:] == "-temp":
+				continue
+
+			# Skip commands that have already been executed.
 			if os.path.exists(AGENT_ROOT + "/commands/" + filename + "-results"):
 				continue
 
@@ -70,34 +76,37 @@
 					command.run()
 					_writeStatus(command, AGENT_ROOT + "/commands/" + filename + "-results")
 	finally:
-		_unlockCommandDir()
+		_unlockAgentDir()
 
 def _writeStatus(command, filename):
 	"""Write a command's status and output."""
 
-	handle = open(filename, "w")
-	handle.write(str(command.status) + " " + str(command.returnCode) + "\n")
-	handle.write(join(command.output, ""))
-	handle.close()
+	try:
+		# We're okay without O_EXCL here, since the agent directory is locked.
+		handle = open(filename + "-temp", "w")
+		handle.write(str(command.status) + " " + str(command.returnCode) + "\n")
+		handle.write(join(command.output, ""))
+		handle.close()
+		os.rename(filename + "-temp", filename)
+	except:
+		# FIXME: should we syslog the data, as an attempt to avoid losing
+		# it? Or should we just leave this, in case it's needed for
+		# troubleshooting?
+		os.unlink(filename + "-temp")
 
-def _lockCommandDir():
+def _lockAgentDir():
 	"""Lock the command directory.
 
 	Returns a boolean indicating whether the directory was successfully
 	locked. Will time out and override the lock file after LOCK_TIMEOUT
 	seconds.
-
-	FIXME: should we use POSIX file locking or O_EXCL to further avoid a
-	race condition? Currently, the cron job only runs once a minute, so only
-	one process should be checking/manipulating the lock at once, but there
-	may be other uses in the future.
 	"""
 
 	if not os.access(LOCKFILE, os.F_OK):
 		# Lock doesn't exist, create it.
-		lockFile = file(LOCKFILE, "w+")
-		lockFile.write(str(os.getpid()) + "\n")
-		lockFile.close()
+		lockFd = os.open(LOCKFILE, os.O_CREAT | os.O_EXCL | os.O_WRONLY)
+		os.write(lockFd, str(os.getpid()) + "\n")
+		os.close(lockFd)
 		return True
 
 	info = os.stat(LOCKFILE)
@@ -106,11 +115,11 @@
 		return False
 
 	# FIXME: log that we're breaking this lock.
-	lockFile = file(LOCKFILE, "w+")
-	lockFile.write(str(os.getpid()) + "\n")
-	lockFile.close()
+	lockFd = os.open(LOCKFILE, os.O_CREAT | os.O_WRONLY | os.O_TRUNC)
+	os.write(lockFd, str(os.getpid()) + "\n")
+	os.close(lockFd)
 	return True
 
-def _unlockCommandDir():
+def _unlockAgentDir():
 	"""Unlock the command directory."""
 	os.remove(LOCKFILE)