Gary Poster has proposed merging lp:~gary/launchpad/bug844631 into lp:launchpad.
Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #844631 in Launchpad itself: "Launchpad should return 503 error pages when database is unavailable" https://bugs.launchpad.net/launchpad/+bug/844631 For more details, see: https://code.launchpad.net/~gary/launchpad/bug844631/+merge/77725 = Overview = This branch adds an error page for when the database is down (such as fastdowntime) or has serious issues. It catches both OperationalError and DisconnectionError because, on experimentation, it seems that the database raises OperationalError if it can't connect initially (such as after an app restart when the db is down), and (as expected) DisconnectionError if it was connected but lost the connection. = UI = The page design is by Huw, except for the status update section, which I had to rewrite because of technical reasons (see below). = Implementation = The registration of the page is straightforward. The branch only has two elements of real interest (and which took up my time). == Testing == The first exciting bit was testing. Stuart suggested that I write integration tests using pgbouncer. This was a bit challenging because it found some edge cases in using the pgbouncer. In particular, you'll see an odd little loop in the test where I get rid of two 500 errors. This was about the best I could do. I talked with him about this for some time and had some alternatives, but we settled on this. I'm not sure what to tell you about it, even though it took a longer time than expected. == Status updates == The second element of this branch that took up my time is the inclusion of the status updates on the page. === Rejected ideas === (Feel free to skip; I feel compelled to share.) Huw initially had done this with a Twitter-supplied widget that had a number of problems, such as relying on http. He found another widget, but it relied on jsonp, which would introduce a security hole for our site (http://en.wikipedia.org/wiki/JSONP#Security_concerns). Matthew proposed reusing the same style of memcached RSS loading that we use on the front page for the blog listings, but that was risky for a page that is supposed to be shown when the system is having problems already, and also would have required poking another hole in our firewall. Robert proposed the interesting idea of having a static page hosted in Apache over http which used jsonp, and redirect the appserver error page to that page. That probably would have worked, but I was hoping for a lighter weight solution. === Implementation: the story === (OK, you can skip this too. This took so long that I feel like talking about it, I guess.) I next explored the Flash xdr method that YUI provides. Identi.ca does not provide a http://identi.ca/crossdomain.xml , which Flash requires. twitter.com and api.twitter.com explicitly have a crossdomain.xml that only allows their own sites to use it. After a bit of digging, I discovered that search.twitter.com does have a friendly crossdomain.xml. I hooked it up and it was working. Unfortunately, it used Flash. Next, I discovered CORS (http://en.wikipedia.org/wiki/Cross-Origin_Resource_Sharing). Yay! This is the answer we want. === Implementation: the meat === (Read this part, please.) I try CORS first. That should work on modern Ubuntu installs. If that fails, I fall back to Flash. If that fails, I give up. This seems to work. Yay. === Testing the JS out yourself === You probably could just shut down PG while you run LP, but I installed a temporary view to look at it. === modified file 'lib/lp/app/browser/configure.zcml' --- lib/lp/app/browser/configure.zcml 2011-09-29 02:32:20 +0000 +++ lib/lp/app/browser/configure.zcml 2011-09-29 18:54:11 +0000 @@ -377,6 +377,15 @@ class="canonical.launchpad.webapp.error.OperationalErrorView" /> + <!-- XXX Remove --> + <browser:page + for="*" + name="disconnected.html" + permission="zope.Public" + template="../templates/launchpad-databaseunavailable.pt" + class="canonical.launchpad.webapp.error.DisconnectionErrorView" + /> + <!-- UnsafeFormGetSubmissionError --> <browser:page for="canonical.launchpad.webapp.interfaces.UnsafeFormGetSubmissionError" == Lint == lint is happy AFAICT except for the usual long line complaints in the lazr config file, but "make lint" is confused and gives me lint on the whole tree, so it's possible I missed something. That's it. Thanks! Gary -- https://code.launchpad.net/~gary/launchpad/bug844631/+merge/77725 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug844631 into lp:launchpad.
=== modified file 'lib/canonical/config/schema-lazr.conf' --- lib/canonical/config/schema-lazr.conf 2011-09-28 03:26:58 +0000 +++ lib/canonical/config/schema-lazr.conf 2011-09-30 15:44:28 +0000 @@ -1211,6 +1211,17 @@ # The number of items to display: homepage_recent_posts_count: 6 +# The URL to the CORS-friendly JSON feed of Launchpad status updates +# that is used on some error pages. +launchpadstatus_json_cors_feed: https://identi.ca/api/statuses/user_timeline/84819.json + +# The URL to the Flash-friendly JSON feed of Launchpad status updates +# that is used on some error pages when the CORS feed does not work. +# Note that this has a slightly different structure than the identi.ca +# feed above: you must get the "results" from this data structure in +# order to get the same results as the identi.ca one. +launchpadstatus_json_flash_feed: https://search.twitter.com/search.json?q=from:launchpadstatus + # The URL of the XML-RPC endpoint that allows querying of feature # flags. This should implement IFeatureFlagApplication. # === added file 'lib/canonical/launchpad/images/identica_logo.png' Binary files lib/canonical/launchpad/images/identica_logo.png 1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/identica_logo.png 2011-09-30 15:44:28 +0000 differ === added file 'lib/canonical/launchpad/images/twitter_logo.png' Binary files lib/canonical/launchpad/images/twitter_logo.png 1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/twitter_logo.png 2011-09-30 15:44:28 +0000 differ === modified file 'lib/canonical/launchpad/webapp/error.py' --- lib/canonical/launchpad/webapp/error.py 2011-09-30 05:51:03 +0000 +++ lib/canonical/launchpad/webapp/error.py 2011-09-30 15:44:28 +0000 @@ -287,3 +287,16 @@ def isSystemError(self): """We don't need to log these errors in the SiteLog.""" return False + + +class DisconnectionErrorView(SystemErrorView): + + response_code = httplib.SERVICE_UNAVAILABLE + reason = u'our database being temporarily offline' + cors_feed = config.launchpad.launchpadstatus_json_cors_feed + flash_feed = config.launchpad.launchpadstatus_json_flash_feed + + +class OperationalErrorView(DisconnectionErrorView): + + reason = u'our database having temporary operational issues' === modified file 'lib/canonical/launchpad/webapp/publication.py' --- lib/canonical/launchpad/webapp/publication.py 2011-08-16 20:38:35 +0000 +++ lib/canonical/launchpad/webapp/publication.py 2011-09-30 15:44:28 +0000 @@ -8,7 +8,6 @@ ] import re -import sys import thread import threading import traceback @@ -19,7 +18,6 @@ URI, ) from psycopg2.extensions import TransactionRollbackError -from storm.database import STATE_DISCONNECTED from storm.exceptions import ( DisconnectionError, IntegrityError, @@ -151,7 +149,7 @@ # exception was added as a result of bug 597324 (message #10 in # particular). return - referrer = request.getHeader('referer') # match HTTP spec misspelling + referrer = request.getHeader('referer') # Match HTTP spec misspelling. if not referrer: raise NoReferrerError('No value for REFERER header') # XXX: jamesh 2007-04-26 bug=98437: @@ -533,8 +531,9 @@ if request.method in ['GET', 'HEAD']: self.finishReadOnlyRequest(txn) elif txn.isDoomed(): - txn.abort() # Sends an abort to the database, even though - # transaction is still doomed. + # Sends an abort to the database, even though transaction + # is still doomed. + txn.abort() else: txn.commit() @@ -736,11 +735,11 @@ if IBrowserRequest.providedBy(request): OpStats.stats['http requests'] += 1 status = request.response.getStatus() - if status == 404: # Not Found + if status == 404: # Not Found OpStats.stats['404s'] += 1 - elif status == 500: # Unhandled exceptions + elif status == 500: # Unhandled exceptions OpStats.stats['500s'] += 1 - elif status == 503: # Timeouts + elif status == 503: # Timeouts OpStats.stats['503s'] += 1 # Increment counters for status code groups. @@ -751,29 +750,6 @@ if is_browser(request) and status_group == '5XXs': OpStats.stats['5XXs_b'] += 1 - # Make sure our databases are in a sane state for the next request. - thread_name = threading.currentThread().getName() - for name, store in getUtility(IZStorm).iterstores(): - try: - assert store._connection._state != STATE_DISCONNECTED, ( - "Bug #504291: Store left in a disconnected state.") - except AssertionError: - # The Store is in a disconnected state. This should - # not happen, as store.rollback() should have been called - # by now. Log an OOPS so we know about this. This - # is Bug #504291 happening. - getUtility(IErrorReportingUtility).raising( - sys.exc_info(), request) - # Repair things so the server can remain operational. - store.rollback() - # Reset all Storm stores when not running the test suite. - # We could reset them when running the test suite but - # that'd make writing tests a much more painful task. We - # still reset the slave stores though to minimize stale - # cache issues. - if thread_name != 'MainThread' or name.endswith('-slave'): - store.reset() - class InvalidThreadsConfiguration(Exception): """Exception thrown when the number of threads isn't set correctly.""" === modified file 'lib/canonical/launchpad/webapp/tests/test_error.py' --- lib/canonical/launchpad/webapp/tests/test_error.py 2011-07-14 05:54:46 +0000 +++ lib/canonical/launchpad/webapp/tests/test_error.py 2011-09-30 15:44:28 +0000 @@ -3,10 +3,27 @@ """Test error views.""" -from canonical.launchpad.webapp.error import SystemErrorView +import urllib2 + +from storm.exceptions import ( + DisconnectionError, + OperationalError, + ) +import transaction + +from canonical.launchpad.webapp.error import ( + DisconnectionErrorView, + OperationalErrorView, + SystemErrorView, + ) from canonical.launchpad.webapp.servers import LaunchpadTestRequest from canonical.testing.layers import LaunchpadFunctionalLayer from lp.testing import TestCase +from lp.testing.fixture import ( + PGBouncerFixture, + Urllib2Fixture, + ) +from lp.testing.matchers import Contains class TestSystemErrorView(TestCase): @@ -29,3 +46,76 @@ self.assertEquals( 'OOPS-1X1', request.response.getHeader('X-Lazr-OopsId', literal=True)) + + +class TestDatabaseErrorViews(TestCase): + + layer = LaunchpadFunctionalLayer + + def getHTTPError(self, url): + try: + urllib2.urlopen(url) + except urllib2.HTTPError, error: + return error + else: + self.fail("We should have gotten an HTTP error") + + def test_disconnectionerror_view_integration(self): + # Test setup. + self.useFixture(Urllib2Fixture()) + bouncer = PGBouncerFixture() + self.useFixture(bouncer) + # Verify things are working initially. + url = 'http://launchpad.dev/' + urllib2.urlopen(url) + # Now break the database, and we get an exception, along with + # our view. + bouncer.stop() + for i in range(2): + # This should not happen ideally, but Stuart is OK with it + # for now. His explanation is that the first request + # makes the PG recognize that the slave DB is + # disconnected, the second one makes PG recognize that the + # master DB is disconnected, and third and subsequent + # requests, as seen below, correctly generate a + # DisconnectionError. Oddly, these are ProgrammingErrors. + self.assertEqual(500, self.getHTTPError(url).code) + error = self.getHTTPError(url) + self.assertEqual(503, error.code) + self.assertThat(error.read(), + Contains(DisconnectionErrorView.reason)) + # We keep seeing the correct exception on subsequent requests. + self.assertEqual(503, self.getHTTPError(url).code) + # When the database is available again, requests succeed. + bouncer.start() + urllib2.urlopen(url) + + def test_disconnectionerror_view(self): + request = LaunchpadTestRequest() + DisconnectionErrorView(DisconnectionError(), request) + self.assertEquals(503, request.response.getStatus()) + + def test_operationalerror_view_integration(self): + # Test setup. + self.useFixture(Urllib2Fixture()) + bouncer = PGBouncerFixture() + self.useFixture(bouncer) + # This is necessary to avoid confusing PG after the stopped bouncer. + transaction.abort() + # Database is down initially, causing an OperationalError. + bouncer.stop() + url = 'http://launchpad.dev/' + error = self.getHTTPError(url) + self.assertEqual(503, error.code) + self.assertThat(error.read(), + Contains(OperationalErrorView.reason)) + # We keep seeing the correct exception on subsequent requests. + self.assertEqual(503, self.getHTTPError(url).code) + # When the database is available again, requests succeed. + bouncer.start() + urllib2.urlopen(url) + + def test_operationalerror_view(self): + request = LaunchpadTestRequest() + OperationalErrorView(OperationalError(), request) + self.assertEquals(503, request.response.getStatus()) === modified file 'lib/lp/app/browser/configure.zcml' --- lib/lp/app/browser/configure.zcml 2011-09-19 14:29:47 +0000 +++ lib/lp/app/browser/configure.zcml 2011-09-30 15:44:28 +0000 @@ -359,6 +359,24 @@ class="canonical.launchpad.webapp.error.OpenIdDiscoveryFailureView" /> + <!-- DisconnectionError --> + <browser:page + for="storm.exceptions.DisconnectionError" + name="index.html" + permission="zope.Public" + template="../templates/launchpad-databaseunavailable.pt" + class="canonical.launchpad.webapp.error.DisconnectionErrorView" + /> + + <!-- OperationalError --> + <browser:page + for="storm.exceptions.OperationalError" + name="index.html" + permission="zope.Public" + template="../templates/launchpad-databaseunavailable.pt" + class="canonical.launchpad.webapp.error.OperationalErrorView" + /> + <!-- UnsafeFormGetSubmissionError --> <browser:page for="canonical.launchpad.webapp.interfaces.UnsafeFormGetSubmissionError" === added file 'lib/lp/app/templates/launchpad-databaseunavailable.pt' --- lib/lp/app/templates/launchpad-databaseunavailable.pt 1970-01-01 00:00:00 +0000 +++ lib/lp/app/templates/launchpad-databaseunavailable.pt 2011-09-30 15:44:28 +0000 @@ -0,0 +1,290 @@ +<html + xmlns="http://www.w3.org/1999/xhtml"; + xmlns:tal="http://xml.zope.org/namespaces/tal"; + xml:lang="en" + lang="en" +> + <head> + <title>Error: database unavailable</title> + + <style type="text/css"> + +html { + background: #fff; + margin: 0; + padding: 0; +} + +body { + font-family: 'UbuntuBeta Regular', Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif; + font-size: 0.75em; + line-height: 1.3em; + color: #000; + margin: 0 0 40px 0; +} + +a { + color: #03a; + text-decoration: none; +} + +h1 { + font-size: 3.7em; +} + +h2, +h3, +h4 { + font-weight: normal; +} + +h2 { + margin: 1.3em 0 1em 0; + font-size: 1.8em; + line-height: 26px; +} + +h3, h4{ + margin: 0; +} + +h4 { + font-size: 1.4em; + color: #333; +} + +p.sub { + color: #333; +} + +p.large { + font-size: 1.2em; +} + +a img { + vertical-align: middle; +} + +.page-center-wrap { + text-align: left; +} +.page-center { + text-align: center; + margin: 0px; + margin: auto; + width: 780px; +} +.page-center-content { + text-align: left; +} + +#header, +#status-feed, +#status-subscribe, +#footer { + padding-left: 50px; + padding-right: 50px; +} + +#header { + border: 1px solid #d7d3cf; + padding-top: 20px; + padding-bottom: 20px; + margin-top: 15%; +} + +#header .graphic { + font-size: 110px; + color: #646464; + float: right; + padding: 100px 80px 0 0; +} + +#header .content { + margin-right: 300px; +} + +#header-shadow { + height: 2px; + background-color: #ebe9e7; +} + +#status-feed { + padding-top: 10px; + width: 450px; +} + +#status-feed h2 { + border-bottom: 1px solid #d7d3cf; + padding-bottom: 0.7em; + margin-bottom: 0.3em; +} + +#status-feed span.datetime { + color: gray; + font-size: smaller; +} + +#status-feed div { + text-align: center; +} + +#status-subscribe img { + margin-right: 3px; +} + +#status-subscribe a:last-child { + margin-left: 20px; +} + +#footer { + border-top: 1px solid #d7d3cf; + margin-top: 40px; + padding-top: 20px; +} + +</style> + + <metal:load-lavascript use-macro="context/@@+base-layout-macros/load-javascript" +/> + <script tal:define=" + revno modules/lp.app.versioninfo/revno | string:unknown; + icingroot string:/+icing/rev${revno}; + yui string:${icingroot}/yui;" + tal:content=" +string:var lp_yui_root = '${yui}'; +lp_status_flash_feed = '${view/flash_feed}'; +lp_status_cors_feed = '${view/cors_feed}'; +" +></script> + <script id="load-status" type="text/javascript"> +LPS.use('node', 'io-xdr', 'json-parse', 'datatype-date', function(Y) { + var tag = function(name) { + return Y.Node.create('<'+name+'/>'); + } + var li = function(text, created_at) { + var datetime = Y.DataType.Date.parse(created_at); + return tag('li') + .append( + tag('span') + .addClass('datetime') + .set('text', Y.DataType.Date.format(datetime, {format: "%F %X %Z"}))) + .append(tag('br')) + .append( + tag('span') + .addClass('text') + .set('text', text)); + } + Y.on('load', function(e){ + var destination = Y.one('#status-feed'); + var _handleSuccess = function(o, get_results) { + var rawJSON = Y.JSON.parse(o.responseText); + var oRSS = get_results(rawJSON); + if (oRSS && oRSS.length) { + destination.one('div').setStyle('display', 'none'); + var l = destination.one('ul'); + for (var i=0; i<Math.min(oRSS.length, 5); i++) { + var entry = oRSS[i]; + l.append(li(entry.text, entry.created_at)); + } + } + }; + var handleTwitterSuccess = function(id, o, a){ + _handleSuccess(o, function(json) {return json.results;}); + }; + var handleIdenticaSuccess = function(id, o, a){ + _handleSuccess(o, function(json) {return json;}); + } + var loadStatusWithFlash = function(){ + var obj = Y.io( + lp_status_flash_feed, + { + method: 'GET', + xdr: {use: 'flash'}, + on: { + success: handleTwitterSuccess, + failure: function (id, o, a){ + Y.log("ERROR " + id + " " + a, "info", "example"); + Y.one('#status-feed div').set( + 'text', '[Sorry, the feed could not load.]'); + } + } + }); + }; + var loadStatusWithCORS = function() { + // CORS: http://www.w3.org/TR/cors/ + // Identi.ca supports it; Twitter does not at this time. + var obj = Y.io( + lp_status_cors_feed, + { + // This is important for Chromium. Otherwise, the XHR will fail. + headers: { 'X-Requested-With': 'disable'}, + on: { + success: handleIdenticaSuccess, + failure: function(id, o, a){ + Y.log("ERROR " + id + " " + a, "info", "example"); + Y.io.transport({id: 'flash', src: lp_yui_root + '/io/io.swf'}); + Y.on('io:xdrReady', loadStatusWithFlash); + } + } + }); + }; + loadStatusWithCORS(); + }, window); +}); + </script> +</head> +<body> + <div class="page-center-wrap"> + <div class="page-center"> + <div class="page-center-content"> + <div id="header"> + <div class="graphic"> + :( + </div> + <div class="content"> + <h1>Uh oh!</h1> + <h4>Something has gone wrong, sorry about this.</h4> + <p>This happens when we are doing an update, in which case + Launchpad will be back in less than five minutes; or + when something else has gone wrong, in which case + someone will have already been notified.</p> + <p class="sub">Technically, this is a 503 error and has + been caused by + <span tal:replace="view/reason">our database + being temporarily offline</span>. + </p> + <p><a href="javascript:window.location.reload()">Reload</a> + this page or try again in a few minutes</p> + </div> + </div> + <div id="header-shadow"></div> + <div id="status-feed"> + <h2>Recent status updates</h2> + <div><img src="/@@/spinner.gif" /></div> + <ul></ul> + </div> + <div id="status-subscribe"> + <h2>Subscribe to updates</h2> + <a href="http://identi.ca/launchpadstatus";> + <img src="/@@/identica_logo.png" alt="Identi.ca" /> + @launchpadstatus + </a> + on Identi.ca + <a href="http://twitter.com/launchpadstatus";> + <img src="/@@/twitter_logo.png" alt="Twitter" /> + @launchpadstatus + </a> + on Twitter + </div> + <div id="footer"> + <a href="http://launchpad.net/";> + <img src="/@@/launchpad-logo-and-name-hierarchy.png" + alt="Launchpad" /> + </a> + </div> + </div> + </div> + </div> +</body> +</html> === modified file 'lib/lp/services/profile/profile.py' --- lib/lp/services/profile/profile.py 2011-09-01 16:25:05 +0000 +++ lib/lp/services/profile/profile.py 2011-09-30 15:44:28 +0000 @@ -210,9 +210,9 @@ assert _profilers.profiler is None actions = get_desired_profile_actions(event.request) _profilers.actions = actions - _profilers.profiling = True if config.profiling.profile_all_requests: actions['callgrind'] = '' + _profilers.profiling = True if actions: if 'sql' in actions: condition = actions['sql'] @@ -222,8 +222,10 @@ if 'show' in actions or available_profilers.intersection(actions): _profilers.profiler = Profiler() _profilers.profiler.start() + _profilers.profiling = True if config.profiling.memory_profile_log: _profilers.memory_profile_start = (memory(), resident()) + _profilers.profiling = True template = PageTemplateFile( os.path.join(os.path.dirname(__file__), 'profile.pt')) === modified file 'lib/lp/testing/fixture.py' --- lib/lp/testing/fixture.py 2011-09-13 14:11:28 +0000 +++ lib/lp/testing/fixture.py 2011-09-30 15:44:28 +0000 @@ -5,6 +5,8 @@ __metaclass__ = type __all__ = [ + 'PGBouncerFixture', + 'Urllib2Fixture', 'ZopeAdapterFixture', 'ZopeEventHandlerFixture', 'ZopeViewReplacementFixture', @@ -18,6 +20,14 @@ Fixture, ) import pgbouncer.fixture +from wsgi_intercept import ( + add_wsgi_intercept, + remove_wsgi_intercept, + ) +from wsgi_intercept.urllib2_intercept import ( + install_opener, + uninstall_opener, + ) from zope.component import ( getGlobalSiteManager, provideHandler, @@ -31,6 +41,7 @@ ) from canonical.config import config +from canonical.testing.layers import wsgi_application class PGBouncerFixture(pgbouncer.fixture.PGBouncerFixture): @@ -173,3 +184,18 @@ self.gsm.adapters.register( (self.context_interface, self.request_interface), Interface, self.name, self.original) + + +class Urllib2Fixture(Fixture): + """Let tests use urllib to connect to an in-process Launchpad. + + Initially this only supports connecting to launchpad.dev because + that is all that is needed. Later work could connect all + sub-hosts (e.g. bugs.launchpad.dev).""" + + def setUp(self): + super(Urllib2Fixture, self).setUp() + add_wsgi_intercept('launchpad.dev', 80, lambda: wsgi_application) + self.addCleanup(remove_wsgi_intercept, 'launchpad.dev', 80) + install_opener() + self.addCleanup(uninstall_opener)
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
