launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-627701 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Permit controlling timeouts via feature flags - a collaboration between Diogo and I.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-627701/+merge/37094
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-627701 into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/doc/timeout.txt'
--- lib/canonical/launchpad/doc/timeout.txt	2010-09-13 04:45:59 +0000
+++ lib/canonical/launchpad/doc/timeout.txt	2010-09-30 02:02:53 +0000
@@ -37,20 +37,19 @@
 (Set the request to have started a few seconds in the past.)
 
     >>> import time
-    >>> from canonical.launchpad.webapp.adapter import set_request_started
-    >>> now = time.time()
-    >>> set_request_started(now-5)
+    >>> from canonical.launchpad.webapp import adapter
+    >>> adapter.set_request_started(time.time()-5)
 
 So the computed timeout should be more or less 5 seconds (10-5).
 
-    >>> timeout_func() - now <= 5
+    >>> timeout_func() <= 5
     True
 
 If the timeout is already expired, a RequestExpired error is raised:
 
     >>> from canonical.launchpad.webapp.adapter import clear_request_started
     >>> clear_request_started()
-    >>> set_request_started(now-12)
+    >>> adapter.set_request_started(time.time()-12)
     >>> timeout_func()
     Traceback (most recent call last):
       ...
@@ -79,6 +78,39 @@
 
     >>> wait_a_little()
 
+= Overriding hard timeouts via FeatureFlags =
+
+It's possible to use FeatureFlags to control the hard timeout. This is used to
+deal with pages that suddenly start performing badly, which are being optimised
+but should not hold back the overall timeout decrease, or for which there are
+only a few specific users and we are willing to have them run for longer
+periods.
+
+    >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+    >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+    >>> from lp.services.features.webapp import ScopesFromRequest
+    >>> from lp.services.features.flags import FeatureController
+    >>> from lp.services.features import per_thread
+
+Install the feature flag to increase the timeout value.
+
+    >>> config.push('timeout', dedent('''\
+    ... [database]
+    ... db_statement_timeout = 10000'''))
+
+    >>> empty_request = LaunchpadTestRequest()
+    >>> per_thread.features = FeatureController(
+    ...     ScopesFromRequest(empty_request).lookup)
+    >>> ignore = getFeatureStore().add(FeatureFlag(
+    ...     scope=u'default', flag=u'hard_timeout', value=u'20000',
+    ...     priority=1))
+
+Now the request can take 20 seconds to complete.
+
+    >>> clear_request_started()
+    >>> adapter.set_request_started(time.time())
+    >>> abs(adapter._get_request_timeout()-20000) < 0.001
+    True
 
 == Clean up ===
 

=== modified file 'lib/canonical/launchpad/webapp/adapter.py'
--- lib/canonical/launchpad/webapp/adapter.py	2010-09-14 04:52:39 +0000
+++ lib/canonical/launchpad/webapp/adapter.py	2010-09-30 02:02:53 +0000
@@ -6,7 +6,7 @@
 
 __metaclass__ = type
 
-import datetime
+import logging
 import os
 import re
 import sys
@@ -47,7 +47,6 @@
 from canonical.config import (
     config,
     DatabaseConfig,
-    dbconfig,
     )
 from canonical.database.interfaces import IRequestExpired
 from canonical.launchpad.interfaces import (
@@ -67,6 +66,7 @@
 from canonical.launchpad.webapp.opstats import OpStats
 from canonical.lazr.utils import get_current_browser_request, safe_hasattr
 from canonical.lazr.timeout import set_default_timeout_function
+from lp.services import features
 from lp.services.timeline.timeline import Timeline
 from lp.services.timeline.requesttimeline import (
     get_request_timeline,
@@ -82,8 +82,6 @@
     'get_request_start_time',
     'get_request_duration',
     'get_store_name',
-    'hard_timeout_expired',
-    'launchpad_default_timeout',
     'soft_timeout_expired',
     'StoreSelector',
     ]
@@ -182,6 +180,12 @@
     if txn is not None:
         _local.commit_logger = CommitLogger(txn)
         txn.registerSynch(_local.commit_logger)
+    # Calculate the hard timeout: needed because featureflags can be used
+    # to control the hard timeout, and they trigger DB access, but our
+    # DB tracers are not safe for reentrant use, so we nmust do this outside
+    # of the SQL stack.
+    _get_request_timeout()
+
 
 def clear_request_started():
     """Clear the request timer.  This function should be called when
@@ -257,10 +261,35 @@
     return now - starttime
 
 
+def _get_request_timeout(now=None, timeout=None):
+    """Get the timeout value in ms for the current request.
+    
+    :param now: Override the result of time.time.
+    :param timeout: A custom timeout in ms.
+    :return None or a time in ms representing the budget to grant the request.
+    """
+    if not getattr(_local, 'enable_timeout', True):
+        return None
+    if timeout is None:
+        timeout = config.database.db_statement_timeout
+        if not getattr(_local, '_getting_timeout_flag', False):
+            _local._getting_timeout_flag = True
+            try:
+                timeout_str = features.getFeatureFlag('hard_timeout')
+            finally:
+                _local._getting_timeout_flag = False
+            if timeout_str:
+                try:
+                    timeout = float(timeout_str)
+                except ValueError:
+                    logging.error('invalid hard timeout flag %r', timeout_str)
+    return timeout
+
+
 def get_request_remaining_seconds(no_exception=False, now=None, timeout=None):
     """Return how many seconds are remaining in the current request budget.
 
-    If timouts are disabled, None is returned. 
+    If timeouts are disabled, None is returned.
 
     :param no_exception: If True, do not raise an error if the request
         is out of time. Instead return a float e.g. -2.0 for 2 seconds over
@@ -269,10 +298,7 @@
     :param timeout: A custom timeout in ms.
     :return: None or a float representing the remaining time budget.
     """
-    if not getattr(_local, 'enable_timeout', True):
-        return None
-    if timeout is None:
-        timeout = config.database.db_statement_timeout
+    timeout = _get_request_timeout(now=now, timeout=timeout)
     if not timeout:
         return None
     duration = get_request_duration(now)