Cleanup session creation code, use close() instead of rollback()

[MatzeB]

Summary

session.rollback() leads to sqlalchemy producing a bunch of additional
queries to compare the objects in the session cache with the database
and rolling the objects in the cache back if necessary. This is a lot
of unnecessary extra database traffic that we do not need there.

[MatzeB]

ping.
@cmatthews: I actually sneaked this change into our internal server and it is running fine for a while now.

[cmatthews]

LGTM. Thanks!

[MatzeB]

Thanks, comitted in r314184

lnt/server/ui/api.py

 import lnt.util.ImportData
 import sqlalchemy
 from flask import current_app, g, Response, make_response, stream_with_context
 from flask import json, jsonify
 from flask import request
 from flask_restful import Resource, abort
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import NoResultFound
 
 from lnt.server.ui.util import convert_revision
+from lnt.server.ui.decorators import in_db
 from lnt.testing import PASS
 from lnt.util import logger
 from functools import wraps
 
 
-def in_db(func):
-    """Extract the database information off the request and attach to
-    particular test suite and database."""
-
-    def wrap(*args, **kwargs):
-        db = kwargs.pop('db')
-        ts = kwargs.pop('ts')
-        g.db_name = db
-        g.testsuite_name = ts
-        g.db_info = current_app.old_config.databases.get(g.db_name)
-        if g.db_info is None:
-            abort(404, message="Invalid database.")
-        request.db = current_app.instance.get_database(g.db_name)
-        request.session = request.db.make_session()
-        # Compute result.
-        result = func(*args, **kwargs)
-
-        # Make sure that any transactions begun by this request are finished.
-        request.session.rollback()
-        return result
-
-    return wrap
-
-
 def requires_auth_token(f):
     @wraps(f)
     def decorated(*args, **kwargs):
         token = request.headers.get("AuthToken", None)
         if not current_app.old_config.api_auth_token or \
            token != current_app.old_config.api_auth_token:
             abort(401, msg="Auth Token must be passed in AuthToken header, "
                            "and included in LNT config.")

lnt/server/ui/decorators.py

 import flask
 from flask import abort
 from flask import current_app, g, render_template
 from flask import request
 
 frontend = flask.Blueprint("lnt", __name__, template_folder="ui/templates/",
                            static_folder="ui/static")
 
 
-# Decorator for implementing per-database routes.
+def _make_db_session(db_name):
+    # Initialize the database parameters on the app globals object.
+    g.db_name = db_name or "default"
+    g.db_info = current_app.old_config.databases.get(g.db_name)
+    if g.db_info is None:
+        abort(404, "Unknown database.")
+    request.db = current_app.instance.get_database(g.db_name)
+    request.session = request.db.make_session()
+
+
 def db_route(rule, **options):
     """
     LNT specific route for endpoints which always refer to some database
     object.
 
     This decorator handles adding the routes for both the default and explicit
     database, as well as initializing the global database information objects.
     """
     def decorator(f):
         def wrap(db_name=None, **args):
-            # Initialize the database parameters on the app globals object.
-            g.db_name = db_name or "default"
-            g.db_info = current_app.old_config.databases.get(g.db_name)
-            if g.db_info is None:
-                abort(404)
-            request.db = current_app.instance.get_database(g.db_name)
-            request.session = request.db.make_session()
-
-            # Compute result.
-            result = f(**args)
-
-            # Make sure that any transactions begun by this request are
-            # finished.
-            request.session.rollback()
-
-            # Return result.
-            return result
+            _make_db_session(db_name)
+            try:
+                return f(**args)
+            finally:
+                request.session.close()
 
         frontend.add_url_rule(rule, f.__name__, wrap, **options)
         frontend.add_url_rule("/db_<db_name>" + rule,
                               f.__name__, wrap, **options)
 
         return wrap
     return decorator
 
 
-# Decorator for implementing per-testsuite routes.
 def v4_route(rule, **options):
     """
     LNT V4 specific route for endpoints which always refer to some testsuite
     object.
     """
-
-    # FIXME: This is manually composed with db_route.
     def decorator(f):
         def wrap(testsuite_name, db_name=None, **args):
-            # Initialize the test suite parameters on the app globals object.
             g.testsuite_name = testsuite_name
-
-            # Initialize the database parameters on the app globals object.
-            g.db_name = db_name or "default"
-            g.db_info = current_app.old_config.databases.get(g.db_name)
-            if g.db_info is None:
-                abort(404)
-            request.db = current_app.instance.get_database(g.db_name)
-            request.session = request.db.make_session()
-
-            # Compute result.
-            result = f(**args)
-
-            # Make sure that any transactions begun by this request are
-            # finished.
-            request.session.rollback()
-
-            # Return result.
-            return result
+            _make_db_session(db_name)
+            try:
+                return f(**args)
+            finally:
+                request.session.close()
 
         frontend.add_url_rule("/v4/<testsuite_name>" + rule,
                               f.__name__, wrap, **options)
         frontend.add_url_rule("/db_<db_name>/v4/<testsuite_name>" + rule,
                               f.__name__, wrap, **options)
 
         return wrap
     return decorator
+
+
+def in_db(func):
+    """Extract the database information off the request and attach to
+    particular test suite and database. Used by the REST api."""
+    def wrap(*args, **kwargs):
+        db = kwargs.pop('db')
+        ts = kwargs.pop('ts')
+        g.testsuite_name = ts
+        _make_db_session(db)
+        try:
+            return func(*args, **kwargs)
+        finally:
+            request.session.close()
+
+    return wrap