[LNT] r309247 - Rework machine creation strategy

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 26 20:46:12 PDT 2017


Author: matze
Date: Wed Jul 26 20:46:12 2017
New Revision: 309247

URL: http://llvm.org/viewvc/llvm-project?rev=309247&view=rev
Log:
Rework machine creation strategy

Currently when submitting a run and the machine data does not match the
previous data, LNT creates a new machine with the same name (but
different id). This was often very confusing to users.

This changes the strategy to reject the submission if the data does not
match the previous data unless the `--update-machine` flag (or the
update_machine post parameters, etc.) is set in which case the new data
overrides the previous machine data.

Adding previously unset keys to the machine will not lead to a rejection
either way. Leaving out previously set keys is fine (but will not remove
the keys).

This new strategy will result in machine names being unique except for
the case of older entries in the database before this change.

Differential Revision: https://reviews.llvm.org/D35598

Added:
    lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_fine.json
    lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_reject.json
Modified:
    lnt/trunk/lnt/lnttool/import_data.py
    lnt/trunk/lnt/lnttool/main.py
    lnt/trunk/lnt/lnttool/viewcomparison.py
    lnt/trunk/lnt/server/db/testsuitedb.py
    lnt/trunk/lnt/server/ui/api.py
    lnt/trunk/lnt/server/ui/templates/submit_run.html
    lnt/trunk/lnt/server/ui/views.py
    lnt/trunk/lnt/util/ImportData.py
    lnt/trunk/lnt/util/ServerUtil.py
    lnt/trunk/tests/lnttool/submit.shtest

Modified: lnt/trunk/lnt/lnttool/import_data.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/lnttool/import_data.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/lnttool/import_data.py (original)
+++ lnt/trunk/lnt/lnttool/import_data.py Wed Jul 26 20:46:12 2017
@@ -20,9 +20,10 @@ import lnt.formats
 @click.option("--quiet", "-q", is_flag=True, help="don't show test results")
 @click.option("--no-email", is_flag=True, help="don't send e-mail")
 @click.option("--no-report", is_flag=True, help="don't generate report")
+ at click.option("--update-machine", is_flag=True, help="Update machine fields")
 def action_import(instance_path, files, database, output_format, commit,
                   show_sql, show_sample_count, show_raw_result, testsuite,
-                  verbose, quiet, no_email, no_report):
+                  verbose, quiet, no_email, no_report, update_machine):
     """import test data into a database"""
     import contextlib
     import lnt.server.instance
@@ -43,7 +44,7 @@ def action_import(instance_path, files,
             result = lnt.util.ImportData.import_and_report(
                 config, database, db, file_name,
                 output_format, testsuite, commit, show_sample_count,
-                no_email, no_report)
+                no_email, no_report, updateMachine=update_machine)
 
             success &= result.get('success', False)
             if quiet:

Modified: lnt/trunk/lnt/lnttool/main.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/lnttool/main.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/lnttool/main.py (original)
+++ lnt/trunk/lnt/lnttool/main.py Wed Jul 26 20:46:12 2017
@@ -178,9 +178,10 @@ def action_showtests():
 @click.argument("url")
 @click.argument("files", nargs=-1, type=click.Path(exists=True), required=True)
 @click.option("--commit", is_flag=True, help="actually commit the data")
+ at click.option("--update-machine", is_flag=True, help="Update machine fields")
 @click.option("--verbose", "-v", is_flag=True,
               help="show verbose test results")
-def action_submit(url, files, commit, verbose):
+def action_submit(url, files, commit, update_machine, verbose):
     """submit a test report to the server"""
     from lnt.util import ServerUtil
     import lnt.util.ImportData
@@ -192,7 +193,8 @@ def action_submit(url, files, commit, ve
         logger.warning("submit called without --commit, " +
                        "your results will not be saved at the server.")
 
-    files = ServerUtil.submitFiles(url, files, commit, verbose)
+    files = ServerUtil.submitFiles(url, files, commit, verbose,
+                                   updateMachine=update_machine)
     for submitted_file in files:
         if verbose:
             lnt.util.ImportData.print_report_result(

Modified: lnt/trunk/lnt/lnttool/viewcomparison.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/lnttool/viewcomparison.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/lnttool/viewcomparison.py (original)
+++ lnt/trunk/lnt/lnttool/viewcomparison.py Wed Jul 26 20:46:12 2017
@@ -84,9 +84,9 @@ def action_view_comparison(report_a, rep
         # Import the two reports.
         with contextlib.closing(config.get_database('default')) as db:
             r = import_and_report(config, 'default', db, report_a, '<auto>',
-                                  testsuite, commit=True)
+                                  testsuite, commit=True, updateMachine=True)
             import_and_report(config, 'default', db, report_b, '<auto>',
-                              testsuite, commit=True)
+                              testsuite, commit=True, updateMachine=True)
 
             # Dispatch another thread to start the webbrowser.
             comparison_url = '%s/v4/nts/2?compare_to=1' % (url,)

Modified: lnt/trunk/lnt/server/db/testsuitedb.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/db/testsuitedb.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/server/db/testsuitedb.py (original)
+++ lnt/trunk/lnt/server/db/testsuitedb.py Wed Jul 26 20:46:12 2017
@@ -763,50 +763,71 @@ class TestSuiteDB(object):
 
         return None
 
-    def _getOrCreateMachine(self, machine_data):
+    def _getOrCreateMachine(self, machine_data, forceUpdate):
         """
-        _getOrCreateMachine(data) -> Machine, bool
+        _getOrCreateMachine(data, forceUpdate) -> Machine
 
         Add or create (and insert) a Machine record from the given machine data
         (as recorded by the test interchange format).
-
-        The boolean result indicates whether the returned record was
-        constructed or not.
         """
 
-        # Convert the machine data into a machine record. We construct the
-        # query to look for any existing machine at the same time as we build
-        # up the record to possibly add.
-        name = machine_data['name']
-        query = self.query(self.Machine).filter(self.Machine.name == name)
-        machine = self.Machine(name)
+        # Convert the machine data into a machine record.
         machine_parameters = machine_data.copy()
-        machine_parameters.pop('name')
-        # Ignore incoming ids; we will create our own.
-        # TODO: Add some API/result so we can send a warning back to the user
-        # that we ignore the id.
+        name = machine_parameters.pop('name')
+        machine = self.Machine(name)
         machine_parameters.pop('id', None)
-
-        # First, extract all of the specified machine fields.
         for item in self.machine_fields:
             value = machine_parameters.pop(item.name, None)
-            query = query.filter(item.column == value)
             machine.set_field(item, value)
-
-        # Convert any remaining machine_parameters into a JSON encoded blob. We
-        # encode this as an array to avoid a potential ambiguity on the key
-        # ordering.
         machine.parameters = machine_parameters
-        query = query.filter(self.Machine.parameters_data ==
-                             machine.parameters_data)
 
-        # Execute the query to see if we already have this machine.
-        existing_machine = query.first()
-        if existing_machine is not None:
-            return existing_machine, False
-        else:
+        # Look for an existing machine.
+        existing_machines = self.query(self.Machine) \
+            .filter(self.Machine.name == name) \
+            .order_by(self.Machine.id.desc()) \
+            .all()
+        if len(existing_machines) == 0:
             self.add(machine)
-            return machine, True
+            return machine
+
+        existing = existing_machines[0]
+
+        # Unfortunately previous LNT versions allowed multiple machines
+        # with the same name to exist, so we should choose the one that
+        # matches best.
+        if len(existing_machines) > 1:
+            for m in existing_machines:
+                if m.parameters == machine.parameters:
+                    existing = m
+                    break
+
+        # Check and potentially update existing machine.
+        # Parameters that were previously unset are added. If a parameter
+        # changed then we update or abort depending on `forceUpdate`.
+        for field in self.machine_fields:
+            existing_value = existing.get_field(field)
+            new_value = machine.get_field(field)
+            if existing_value is None:
+                existing.set_field(field, new_value)
+            elif existing_value != new_value:
+                if not forceUpdate:
+                    raise ValueError("'%s' on machine '%s' changed." %
+                                     (field.name, name))
+                else:
+                    existing.set_field(field, new_value)
+        existing_parameters = existing.parameters
+        for key, value in machine.parameters.items():
+            existing_value = existing_parameters.get(key, None)
+            if existing_value is None:
+                existing_parameters[key] = value
+            elif existing_value != value:
+                if not forceUpdate:
+                    raise ValueError("'%s' on machine '%s' changed." %
+                                     (key, name))
+                else:
+                    existing_parameters[key] = value
+        existing.parameters = existing_parameters
+        return existing
 
     def _getOrCreateOrder(self, run_parameters):
         """
@@ -979,7 +1000,8 @@ class TestSuiteDB(object):
                     else:
                         sample.set_field(field, value)
 
-    def importDataFromDict(self, data, commit, config=None):
+    def importDataFromDict(self, data, commit, config=None,
+                           updateMachine=False):
         """
         importDataFromDict(data) -> bool, Run
 
@@ -989,9 +1011,7 @@ class TestSuiteDB(object):
         The boolean result indicates whether the returned record was
         constructed or not (i.e., whether the data was a duplicate submission).
         """
-
-        # Construct the machine entry.
-        machine, inserted = self._getOrCreateMachine(data['machine'])
+        machine = self._getOrCreateMachine(data['machine'], updateMachine)
 
         # Construct the run entry.
         run, inserted = self._getOrCreateRun(data['run'], machine)

Modified: lnt/trunk/lnt/server/ui/api.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/ui/api.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/server/ui/api.py (original)
+++ lnt/trunk/lnt/server/ui/api.py Wed Jul 26 20:46:12 2017
@@ -277,8 +277,10 @@ class Runs(Resource):
         """Add a new run into the lnt database"""
         db = request.get_db()
         data = request.data
+        updateMachine = request.values.get('update_machine', False)
         result = lnt.util.ImportData.import_from_string(
-            current_app.old_config, g.db_name, db, g.testsuite_name, data)
+            current_app.old_config, g.db_name, db, g.testsuite_name, data,
+            updateMachine=updateMachine)
 
         new_url = ('%sapi/db_%s/v4/%s/runs/%s' %
                    (request.url_root, g.db_name, g.testsuite_name,

Modified: lnt/trunk/lnt/server/ui/templates/submit_run.html
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/ui/templates/submit_run.html?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/server/ui/templates/submit_run.html (original)
+++ lnt/trunk/lnt/server/ui/templates/submit_run.html Wed Jul 26 20:46:12 2017
@@ -20,6 +20,12 @@
 <option value="1">1</option>
 </select><br/>
 
+<p><b>Update Machine:</b><br/>
+<select name="update_machine">
+<option selected="selected" value="0">0</option>
+<option value="1">1</option>
+</select><br/>
+
 <p><input type="submit" name="submit" value="Submit">
 </form>
 

Modified: lnt/trunk/lnt/server/ui/views.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/ui/views.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/server/ui/views.py (original)
+++ lnt/trunk/lnt/server/ui/views.py Wed Jul 26 20:46:12 2017
@@ -104,6 +104,7 @@ def _do_submit():
     input_file = request.files.get('file')
     input_data = request.form.get('input_data')
     commit = int(request.form.get('commit', 0)) != 0
+    updateMachine = int(request.form.get('update_machine', 0)) != 0
 
     if input_file and not input_file.content_length:
         input_file = None
@@ -143,7 +144,7 @@ def _do_submit():
 
     result = lnt.util.ImportData.import_from_string(
         current_app.old_config, g.db_name, db, g.testsuite_name, data_value,
-        commit=commit)
+        commit=commit, updateMachine=updateMachine)
 
     # It is nice to have a full URL to the run, so fixup the request URL
     # here were we know more about the flask instance.

Modified: lnt/trunk/lnt/util/ImportData.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/util/ImportData.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/util/ImportData.py (original)
+++ lnt/trunk/lnt/util/ImportData.py Wed Jul 26 20:46:12 2017
@@ -13,7 +13,8 @@ import time
 
 def import_and_report(config, db_name, db, file, format, ts_name,
                       commit=False, show_sample_count=False,
-                      disable_email=False, disable_report=False):
+                      disable_email=False, disable_report=False,
+                      updateMachine=False):
     """
     import_and_report(config, db_name, db, file, format, ts_name,
                       [commit], [show_sample_count],
@@ -88,7 +89,8 @@ def import_and_report(config, db_name, d
                                (data_schema, ts_name))
             return result
 
-        success, run = ts.importDataFromDict(data, commit, config=db_config)
+        success, run = ts.importDataFromDict(data, commit, config=db_config,
+                                             updateMachine=updateMachine)
     except KeyboardInterrupt:
         raise
     except Exception as e:
@@ -157,7 +159,8 @@ def import_and_report(config, db_name, d
             shadow_result = import_and_report(config, shadow_name,
                                               shadow_db, file, format, ts_name,
                                               commit, show_sample_count,
-                                              disable_email, disable_report)
+                                              disable_email, disable_report,
+                                              updateMachine)
 
             # Append the shadow result to the result.
             result['shadow_result'] = shadow_result
@@ -306,7 +309,8 @@ def print_report_result(result, out, err
         print >>out, kind, ":", count
 
 
-def import_from_string(config, db_name, db, ts_name, data, commit=True):
+def import_from_string(config, db_name, db, ts_name, data, commit=True,
+                       updateMachine=False):
     # Stash a copy of the raw submission.
     #
     # To keep the temporary directory organized, we keep files in
@@ -334,5 +338,5 @@ def import_from_string(config, db_name,
     # should at least reject overly large inputs.
 
     result = lnt.util.ImportData.import_and_report(config, db_name, db,
-            path, '<auto>', ts_name, commit)
+            path, '<auto>', ts_name, commit, updateMachine=updateMachine)
     return result

Modified: lnt/trunk/lnt/util/ServerUtil.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/util/ServerUtil.py?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/lnt/util/ServerUtil.py (original)
+++ lnt/trunk/lnt/util/ServerUtil.py Wed Jul 26 20:46:12 2017
@@ -28,10 +28,11 @@ def _show_json_error(reply):
     if message:
         sys.stderr.write(message + '\n')
 
-def submitFileToServer(url, file, commit):
+def submitFileToServer(url, file, commit, updateMachine):
     with open(file, 'rb') as f:
-        values = { 'input_data' : f.read(),
-                   'commit' : ("0","1")[not not commit] }
+        values = {'input_data' : f.read(),
+                  'commit' : "1" if commit else "0",
+                  'update_machine': "1" if updateMachine else "0"}
     headers = {'Accept': 'application/json'}
     data = urllib.urlencode(values)
     try:
@@ -58,7 +59,7 @@ def submitFileToServer(url, file, commit
     return reply
 
 
-def submitFileToInstance(path, file, commit):
+def submitFileToInstance(path, file, commit, updateMachine=False):
     # Otherwise, assume it is a local url and submit to the default database
     # in the instance.
     instance = lnt.server.instance.Instance.frompath(path)
@@ -69,24 +70,24 @@ def submitFileToInstance(path, file, com
             raise ValueError("no default database in instance: %r" % (path,))
         return lnt.util.ImportData.import_and_report(
             config, db_name, db, file, format='<auto>', ts_name='nts',
-            commit=commit)
+            commit=commit, updateMachine=updateMachine)
 
 
-def submitFile(url, file, commit, verbose):
+def submitFile(url, file, commit, verbose, updateMachine=False):
     # If this is a real url, submit it using urllib.
     if '://' in url:
-        result = submitFileToServer(url, file, commit)
+        result = submitFileToServer(url, file, commit, updateMachine)
         if result is None:
             return
     else:
-        result = submitFileToInstance(url, file, commit)
+        result = submitFileToInstance(url, file, commit, updateMachine)
     return result
 
 
-def submitFiles(url, files, commit, verbose):
+def submitFiles(url, files, commit, verbose, updateMachine=False):
     results = []
     for file in files:
-        result = submitFile(url, file, commit, verbose)
+        result = submitFile(url, file, commit, verbose, updateMachine)
         if result:
             results.append(result)
     return results

Added: lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_fine.json
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_fine.json?rev=309247&view=auto
==============================================================================
--- lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_fine.json (added)
+++ lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_fine.json Wed Jul 26 20:46:12 2017
@@ -0,0 +1,29 @@
+{
+	"Machine": {
+		"Info": {
+			"hw.activecpu": "4",
+			"hostname": "test.local"
+		},
+		"Name": "some-compile-suite-machine"
+	},
+	"Run": {
+		"End Time": "2017-07-06 15:37:08",
+		"Start Time": "2017-07-06 15:05:23",
+		"Info": {
+			"__report_version__": "1",
+			"run_order": "663360",
+			"tag": "compile"
+		}
+	},
+	"Tests": [
+        {
+            "Data": [
+                11.601326,
+                11.411566,
+                11.490528
+            ],
+            "Info": {},
+            "Name": "compile.build/Adium-1.5.7(config='Debug',j=1).user"
+        }
+	]
+}

Added: lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_reject.json
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_reject.json?rev=309247&view=auto
==============================================================================
--- lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_reject.json (added)
+++ lnt/trunk/tests/lnttool/Inputs/compile_submission_machine_diff_reject.json Wed Jul 26 20:46:12 2017
@@ -0,0 +1,29 @@
+{
+	"Machine": {
+		"Info": {
+			"hw.activecpu": "1",
+			"machdep.cpu.vendor": "GenuineIntel"
+		},
+		"Name": "some-compile-suite-machine"
+	},
+	"Run": {
+		"End Time": "2017-07-06 15:37:08",
+		"Start Time": "2017-07-06 15:05:23",
+		"Info": {
+			"__report_version__": "1",
+			"run_order": "663400",
+			"tag": "compile"
+		}
+	},
+	"Tests": [
+        {
+            "Data": [
+                13.601326,
+                13.411566,
+                13.490528
+            ],
+            "Info": {},
+            "Name": "compile.build/Adium-1.5.7(config='Debug',j=1).user"
+        }
+	]
+}

Modified: lnt/trunk/tests/lnttool/submit.shtest
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/lnttool/submit.shtest?rev=309247&r1=309246&r2=309247&view=diff
==============================================================================
--- lnt/trunk/tests/lnttool/submit.shtest (original)
+++ lnt/trunk/tests/lnttool/submit.shtest Wed Jul 26 20:46:12 2017
@@ -101,3 +101,39 @@ lnt submit "http://localhost:9091/db_def
 # CHECK-ERRORS: lnt server error: import failure: machine
 # ...
 # CHECK-ERRORS: KeyError: 'machine'
+lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_reject.json" >> "${OUTPUT_DIR}/submit_errors.txt" 2>&1
+# CHECK-ERRORS: lnt server error: import failure: 'hw.activecpu' on machine 'some-compile-suite-machine' changed.
+# ...
+# ValueError: 'hw.activecpu' on machine 'some-compile-suite-machine' changed.
+
+
+
+# Adding extra fields to the machine in a submission is fine.
+lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_fine.json" -v > "${OUTPUT_DIR}/submit_compile_machine_diff.txt"
+# RUN: FileCheck %s --check-prefix=CHECK-MACHINEDIFF < %T/submit_compile_machine_diff.txt
+#
+# CHECK-MACHINEDIFF: Imported Data
+# CHECK-MACHINEDIFF: -------------
+# CHECK-MACHINEDIFF-NOT: Added Machines
+# CHECK-MACHINEDIFF: Added Runs    : 1
+# CHECK-MACHINEDIFF-NOT: Added Machines
+#
+# CHECK-MACHINEDIFF: Results
+# CHECK-MACHINEDIFF: ----------------
+# CHECK-MACHINEDIFF: PASS : 9
+# CHECK-MACHINEDIFF: Results available at: http://localhost:9091/db_default/v4/compile/6
+
+# Test updating existing machine
+lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_reject.json" --update-machine -v > "${OUTPUT_DIR}/submit_compile_machine_update.txt"
+# RUN: FileCheck %s --check-prefix=CHECK-UPDATEMACHINE < %T/submit_compile_machine_update.txt
+#
+# CHECK-UPDATEMACHINE: Imported Data
+# CHECK-UPDATEMACHINE: -------------
+# CHECK-UPDATEMACHINE-NOT: Added Machines
+# CHECK-UPDATEMACHINE: Added Runs    : 1
+# CHECK-UPDATEMACHINE-NOT: Added Machines
+#
+# CHECK-UPDATEMACHINE: Results
+# CHECK-UPDATEMACHINE: ----------------
+# CHECK-UPDATEMACHINE: PASS : 9
+# CHECK-UPDATEMACHINE: Results available at: http://localhost:9091/db_default/v4/compile/7




More information about the llvm-commits mailing list