launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #433888 in Launchpad itself: "branch path mapping has no safeguard against exceeding the critical threshold"
  https://bugs.launchpad.net/launchpad/+bug/433888

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-433888/+merge/57617

Set a request up around line rewrites in the branch rewriter. This will cause the timeout code to use the configured db time (and I have a separate prod configs branch to add a branchrewriter config to set that). The combination will let us avoid huge backlogs if the db is busy or down - instead we'll return NULL [and that will tell apache something is wrong].
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-433888/+merge/57617
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-433888 into lp:launchpad.

=== modified file 'lib/lp/codehosting/rewrite.py'
--- lib/lp/codehosting/rewrite.py	2011-04-04 04:14:02 +0000
+++ lib/lp/codehosting/rewrite.py	2011-04-14 02:22:32 +0000
@@ -10,6 +10,7 @@
 from zope.component import getUtility
 
 from canonical.config import config
+from canonical.launchpad.webapp.adapter import set_request_started, clear_request_started
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
 from lp.codehosting.vfs import branch_id_to_path
@@ -92,25 +93,31 @@
         # Codebrowse generates references to its images and stylesheets
         # starting with "/static", so pass them on unthinkingly.
         T = time.time()
-        cached = None
-        if resource_location.startswith('/static/'):
-            r = self._codebrowse_url(resource_location)
-            cached = 'N/A'
-        else:
-            branch_id, trailing, cached = self._getBranchIdAndTrailingPath(
-                resource_location)
-            if branch_id is None:
-                if resource_location.startswith('/' + BRANCH_ID_ALIAS_PREFIX):
-                    r = 'NULL'
-                else:
-                    r = self._codebrowse_url(resource_location)
+        # Tell the webapp adapter that we are in a request, so that DB
+        # statement timeouts will be applied.
+        set_request_started()
+        try:
+            cached = None
+            if resource_location.startswith('/static/'):
+                r = self._codebrowse_url(resource_location)
+                cached = 'N/A'
             else:
-                if trailing.startswith('/.bzr'):
-                    r = urlutils.join(
-                        config.codehosting.internal_branch_by_id_root,
-                        branch_id_to_path(branch_id), trailing[1:])
+                branch_id, trailing, cached = self._getBranchIdAndTrailingPath(
+                    resource_location)
+                if branch_id is None:
+                    if resource_location.startswith('/' + BRANCH_ID_ALIAS_PREFIX):
+                        r = 'NULL'
+                    else:
+                        r = self._codebrowse_url(resource_location)
                 else:
-                    r = self._codebrowse_url(resource_location)
+                    if trailing.startswith('/.bzr'):
+                        r = urlutils.join(
+                            config.codehosting.internal_branch_by_id_root,
+                            branch_id_to_path(branch_id), trailing[1:])
+                    else:
+                        r = self._codebrowse_url(resource_location)
+        finally:
+            clear_request_started()
         self.logger.info(
             "%r -> %r (%fs, cache: %s)",
             resource_location, r, time.time() - T, cached)