Prevent uploading results when affected rankings exist.

Reject uploading if rankings exist for the given context, since
that would require a recomputation of these rankings.

Note that this check if sufficient as long as we assume that results
are uploaded in an order that is consistent with the order of the
corresponding SHA-1s in the branch in question (i.e. that results will
never be uploaded for a SHA-1 that precedes (is "older" than) a SHA-1
for which results already exist). This assumption ensures that results for
a new SHA-1 will not alter the input for any existing ranking.

3 files changed, 63 insertions, 29 deletions

diff --git a/scripts/finalizeresults.py b/scripts/finalizeresults.py
index 2ff9a45..4e4b0d4 100755
--- a/scripts/finalizeresults.py
+++ b/scripts/finalizeresults.py
@@ -198,10 +198,10 @@ def updateRankings(host_id, platform_id, branch_id, sha12_id, context2_id):
     sys.stdout.flush()
 
 
-    # Rankings will normally be computed once a week for each
+    # Rankings will normally be computed once a day for each
     # host/platform/branch combination (note the tradeoff between update
     # frequency and database size):
-    ranking_interval = 3600 * 24 * 7 # secs in a week
+    ranking_interval = 3600 * 24 # secs in a day
 
     # Rankings will be updated if at least one of the following
     # conditions eventually becomes True:
@@ -222,15 +222,16 @@ def updateRankings(host_id, platform_id, branch_id, sha12_id, context2_id):
             target_timestamp = getFirstUploadTimestamp(snapshots, sha12_id)
             if target_timestamp < 0:
                 print (
-                "failed to extract target_timestamp (error in command-line " +
-                "args?)")
+                "error: failed to extract target_timestamp "
+                "(error in command-line args?)")
                 sys.exit(1)
+
             interval_cond = (
                 (target_timestamp - last_ranking_timestamp) > ranking_interval)
 
     if not (force_cond or empty_cond or interval_cond):
         print (
-            "not updating rankings ('force', 'empty', and 'interval' " +
+            "not updating rankings ('force', 'empty', and 'interval' "
             "conditions all failed)")
         return
 
@@ -283,7 +284,7 @@ def updateRankings(host_id, platform_id, branch_id, sha12_id, context2_id):
     # register the ranking positions in the database:
     context1_id = getContext(host_id, platform_id, branch_id, snapshots[0][0])
     if context1_id == -1:
-        print "fatal error: failed to find context for start snapshot"
+        print "error: failed to find context for start snapshot"
         sys.exit(1)
     nameToIndex = { "QS": 3, "LCSSR": 4, "LCSSI": 5, "LCSS1R": 6, "LCSS1I": 7 }
     for name in nameToIndex:
@@ -311,7 +312,7 @@ if (not ("db" in options and "host" in options and "platform" in options and
     sys.exit(0)
 
 if not isValidSHA1(options["sha1"]):
-    print "invalid SHA-1:", options["sha1"]
+    print "error: invalid SHA-1:", options["sha1"]
     sys.exit(1)
 
 setDatabase(
@@ -321,24 +322,24 @@ setDatabase(
 
 host_id = textToId("host", options["host"])
 if host_id == -1:
-    print "no such host:", options["host"]
+    print "error: no such host:", options["host"]
     sys.exit(1)
 platform_id = textToId("platform", options["platform"])
 if platform_id == -1:
-    print "no such platform:", options["platform"]
+    print "error: no such platform:", options["platform"]
     sys.exit(1)
 branch_id = textToId("branch", options["branch"])
 if branch_id == -1:
-    print "no such branch:", options["branch"]
+    print "error: no such branch:", options["branch"]
     sys.exit(1)
 sha12_id = textToId("sha1", options["sha1"])
 if sha12_id == -1:
-    print "no such SHA-1:", options["sha1"]
+    print "error: no such SHA-1:", options["sha1"]
     sys.exit(1)
 
 context2_id = getContext(host_id, platform_id, branch_id, sha12_id)
 if context2_id == -1:
-    print "no results found for this host/platform/branch/SHA-1 combination"
+    print "error: no results found for this context"
     sys.exit(1)
 
 
diff --git a/scripts/misc.py b/scripts/misc.py
index 92d4b2f..8d0be5c 100644
--- a/scripts/misc.py
+++ b/scripts/misc.py
@@ -44,29 +44,32 @@ def textToId(table, text):
     return id_;
 
 # ### 2 B DOCUMENTED!
-def metricIdToLowerIsBetter(metricId):
+# Maybe also rename to lowerIsBetter() ? (but note that a global function with
+# that name already exists in uploadresults.py)
+def metricIdToLowerIsBetter(metric_id):
     global metricIdToLIBCache
     if not 'metricIdToLIBCache' in globals():
         metricIdToLIBCache = {}
 
-    if metricId in metricIdToLIBCache:
-        return metricIdToLIBCache[metricId];
+    if metric_id in metricIdToLIBCache:
+        return metricIdToLIBCache[metric_id];
 
     lib = execQuery(
-        "SELECT lowerIsBetter FROM metric WHERE id = " + str(metricId) +
+        "SELECT lowerIsBetter FROM metric WHERE id = " + str(metric_id) +
         ";")[0][0]
-    metricIdToLIBCache[metricId] = lib;
+    metricIdToLIBCache[metric_id] = lib;
 
     return lib;
 
-def getContext(host, platform, branch, sha1):
+# Returns the non-negative ID of the given context, or -1 if not found.
+def getContext(host_id, platform_id, branch_id, sha1_id):
     result = execQuery("SELECT id FROM context"
                 " WHERE hostId = %d"
                 " AND platformId = %d"
                 " AND branchId = %d"
                 " AND sha1Id = %d"
                 "LIMIT 1;"
-                % (host, platform, branch, sha1))
+                % (host_id, platform_id, branch_id, sha1_id))
     if len(result):
         return result[0][0]
     return -1
diff --git a/scripts/uploadresults.py b/scripts/uploadresults.py
index 7f9c7f9..ab9568e 100755
--- a/scripts/uploadresults.py
+++ b/scripts/uploadresults.py
@@ -3,7 +3,7 @@
 import sys
 from xml.dom.minidom import parse
 from dbaccess import setDatabase, execQuery, commit
-from misc import getOptions, isValidSHA1, getContext
+from misc import getOptions, textToId, isValidSHA1, getContext
 
 
 # --- BEGIN Global functions ----------------------------------------------
@@ -193,9 +193,7 @@ def findOrInsertId(table, value, *args):
 
 
 # Uploads a set of results to the database.
-def uploadToDatabase(dbhost, dbport, db, host, platform, branch, sha1, results):
-
-    setDatabase(dbhost, dbport, db)
+def uploadToDatabase(host, platform, branch, sha1, results):
 
     # Append a row to the 'upload' table (to record this upload event) ...
     execQuery("INSERT INTO upload DEFAULT VALUES;", False)
@@ -240,6 +238,21 @@ def uploadToDatabase(dbhost, dbport, db, host, platform, branch, sha1, results):
     # Write to database:
     commit()
 
+
+# Returns True iff rankings exist for the given context.
+def rankingsExist(host, platform, branch, sha1):
+    context_id = getContext(
+        textToId('host', host),
+        textToId('platform', platform),
+        textToId('branch', branch),
+        textToId('sha1', sha1))
+
+    matches = execQuery(
+        "SELECT id FROM ranking WHERE context2Id = %d LIMIT 1;" % context_id)
+
+    return len(matches) > 0
+
+
 # --- END Global functions ----------------------------------------------
 
 
@@ -257,20 +270,37 @@ if (not ("db" in options and "host" in options and "platform" in options and
     sys.exit(0)
 
 if not isValidSHA1(options["sha1"]):
-    print "invalid SHA-1:", options["sha1"]
+    print "error: invalid SHA-1:", options["sha1"]
+    sys.exit(1)
+
+setDatabase(
+    options["dbhost"] if "dbhost" in options else None,
+    options["dbport"] if "dbport" in options else None,
+    options["db"])
+
+# Reject uploading if rankings exist for this context, since that would
+# require a recomputation of the rankings.
+#
+# (Note that this check if sufficient as long as we assume that results
+# are uploaded in an order that is consistent with the order of the
+# corresponding SHA-1s in the branch in question (i.e. that results will
+# never be uploaded for a SHA-1 that precedes (is "older" than) a SHA-1 for
+# which results already exist). This assumption ensures that results for a
+# new SHA-1 will not alter the input for any existing ranking.)
+if rankingsExist(
+    options["host"], options["platform"], options["branch"], options["sha1"]):
+    print "error: this context has already been finalized"
     sys.exit(1)
 
 try:
     results = extractResults(options["file"])
 except BaseException as e:
-    print "failed to parse XML file:", e.args[0]
+    print "error: failed to parse XML file:", e.args[0]
     sys.exit(1)
 
 uploadToDatabase(
-    options["dbhost"] if "dbhost" in options else None,
-    options["dbport"] if "dbport" in options else None,
-    options["db"], options["host"], options["platform"], options["branch"],
-    options["sha1"], results)
+    options["host"], options["platform"], options["branch"], options["sha1"],
+    results)
 
 print "uploading done"