launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:rearrange-opstats-timeout into launchpad:master.

Commit message:
Move timeout statistics to handleException

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #636804 in Launchpad itself: "request expired being raised does not increased the opstats timeout count"
  https://bugs.launchpad.net/launchpad/+bug/636804

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/403817

This is cleaner than doing it in the timeout tracer.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:rearrange-opstats-timeout into launchpad:master.

diff --git a/lib/lp/services/webapp/adapter.py b/lib/lp/services/webapp/adapter.py
index a707e11..0eb52fb 100644
--- a/lib/lp/services/webapp/adapter.py
+++ b/lib/lp/services/webapp/adapter.py
@@ -625,9 +625,6 @@ class LaunchpadTimeoutTracer(PostgresTimeoutTracer):
             super(LaunchpadTimeoutTracer, self).connection_raw_execute(
                 connection, raw_cursor, statement, params)
         except (RequestExpired, TimeoutError):
-            # XXX: This code does not belong here - see bug=636804.
-            # Robert Collins 20100913.
-            OpStats.stats['timeouts'] += 1
             # XXX bug=636801 Robert Colins 20100914 This is duplicated
             # from the statement tracer, because the tracers are not
             # arranged in a stack rather a queue: the done-code in the
@@ -654,7 +651,6 @@ class LaunchpadTimeoutTracer(PostgresTimeoutTracer):
         if not isinstance(connection._database, LaunchpadDatabase):
             return
         if isinstance(error, QueryCanceledError):
-            OpStats.stats['timeouts'] += 1
             raise LaunchpadTimeoutError(statement, params, error)
 
     def get_remaining_time(self):
diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index b0dabea..e832679 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -25,6 +25,7 @@ from storm.database import STATE_DISCONNECTED
 from storm.exceptions import (
     DisconnectionError,
     IntegrityError,
+    TimeoutError,
     )
 from storm.zope.interfaces import IZStorm
 import transaction
@@ -696,6 +697,9 @@ class LaunchpadBrowserPublication(
         if isinstance(exc_info[1], DisconnectionError):
             getUtility(IErrorReportingUtility).raising(exc_info, request)
 
+        if isinstance(exc_info[1], (da.RequestExpired, TimeoutError)):
+            OpStats.stats['timeouts'] += 1
+
         def should_retry(exc_info):
             if not retry_allowed:
                 return False