From 9c92a2bd86f93a21b78b4d3d93f29158504b0bbb Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sat, 11 Jun 2016 12:02:47 +0100 Subject: [PATCH 1/3] Stop AJAX updates queuing requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `updateContent` module updates a section of the page based on an AJAX request. The current implementation will poll every `x` seconds. If the server takes a long time to respond, this results in a the browser queuing up requests. If a particular endpoint is slow, this can result in one client making enough requests to slow down the whole server, to the point where it gets removed from the loadbalancer, eventually bringing the whole site down. This commit rewrites the module so that it queues up the render operations, not the requests. There is one queue per endpoint, so for `http://example.com/endpoint.json`: 1. Queue is empty ```javascript { 'http://example.com/endpoint.json': [] } ``` 2. Inital re-render is put on the queue… ```javascript { 'http://example.com/endpoint.json': [ function render(){…} ] } ``` …AJAX request fires ``` GET http://example.com/endpoint.json ``` 3. Every `x` seconds, another render operation is put on the queue ```javascript { 'http://example.com/endpoint.json': [ function render(){…}, function render(){…}, function render(){…} ] } ``` 4. AJAX request returns queue is flushed by executing each queued render function in sequence ```javascript render(response); render(response); render(response); ``` ```javascript { 'http://example.com/endpoint.json': [] } ``` 5. Repeat This means that, at most, the AJAX requests will never fire more than once every `x` seconds, where `x` defaults to `1.5`. --- app/assets/javascripts/updateContent.js | 64 ++++++++++++------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/app/assets/javascripts/updateContent.js b/app/assets/javascripts/updateContent.js index 0a20bca93..c7e51be40 100644 --- a/app/assets/javascripts/updateContent.js +++ b/app/assets/javascripts/updateContent.js @@ -1,48 +1,44 @@ -(function(GOVUK, Modules) { +(function(Modules) { "use strict"; - GOVUK.timeCache = {}; - GOVUK.resultCache = {}; - + var queues = {}; var dd = new diffDOM(); - let getter = function(resource, interval, render) { + var getRenderer = $component => response => dd.apply( + $component.get(0), + dd.diff($component.get(0), $(response[$component.data('key')]).get(0)) + ); - if ( - GOVUK.resultCache[resource] && - (Date.now() < GOVUK.timeCache[resource]) - ) { - render(GOVUK.resultCache[resource]); - } else { - GOVUK.timeCache[resource] = Date.now() + interval; - $.get( - resource, - response => render(GOVUK.resultCache[resource] = response) - ); - } + var getQueue = resource => ( + queues[resource] = queues[resource] || [] + ); + var flushQueue = function(queue, response) { + while(queue.length) queue.shift()(response); }; - let poller = (resource, key, $component, interval) => () => getter( - resource, interval, response => dd.apply( - $component.get(0), - dd.diff($component.get(0), $(response[key]).get(0)) - ) - ); + var poll = function(renderer, resource, queue, interval) { + + if (queue.push(renderer) === 1) $.get( + resource, + response => flushQueue(queue, response) + ); + + setTimeout( + () => poll(...arguments), interval + ); + + }; Modules.UpdateContent = function() { - this.start = function(component) { + this.start = component => poll( + getRenderer($(component)), + $(component).data('resource'), + getQueue($(component).data('resource')), + ($(component).data('interval-seconds') || 1.5) * 1000 + ); - const $component = $(component); - interval = ($(component).data("interval-seconds") * 1000) || 1500; - - setInterval( - poller($component.data('resource'), $component.data('key'), $component, interval), - interval / 5 - ); - - }; }; -})(window.GOVUK, window.GOVUK.Modules); +})(window.GOVUK.Modules); From 940170159a227e544fd9b6aa796ff08671d340c3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 12 Jun 2016 08:40:56 +0100 Subject: [PATCH 2/3] Handle AJAX errors when updating content With the change in implementation in the previous commit, any error (eg server responds with `500`) would cause the page to not be updated again. This is better than the previous implementation, whereby the browser would re-request as fast as it could until it got a successful response. This commit handles errors by clearing the render queue if the server returns an error. So: - Any updates that would have been performed based on this request are dropped - Subsequent updates will be attempted as if it was the first load of the page, ie after a delay of `x` seconds --- app/assets/javascripts/updateContent.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/updateContent.js b/app/assets/javascripts/updateContent.js index c7e51be40..41dd41ff5 100644 --- a/app/assets/javascripts/updateContent.js +++ b/app/assets/javascripts/updateContent.js @@ -17,11 +17,16 @@ while(queue.length) queue.shift()(response); }; + var clearQueue = queue => (queue.length = 0); + var poll = function(renderer, resource, queue, interval) { - if (queue.push(renderer) === 1) $.get( - resource, + if (queue.push(renderer) === 1) $.ajax( + resource + ).done( response => flushQueue(queue, response) + ).fail( + () => clearQueue(queue) ); setTimeout( From 33b18cedebf8cf82bc0347db9de12e2c8e984e37 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sat, 11 Jun 2016 12:04:05 +0100 Subject: [PATCH 3/3] Revert "Comment out ajax queries to update content on frontend." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 651584d05685f12299a7a4a4543fd644e5848b74. Should be safe to turn the AJAX back on now it’s not going to denial-of-service any slow pages. --- app/templates/partials/jobs/count.html | 4 ++-- app/templates/partials/jobs/notifications.html | 4 ++-- app/templates/partials/jobs/status.html | 4 ++-- app/templates/views/dashboard/today.html | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/templates/partials/jobs/count.html b/app/templates/partials/jobs/count.html index b64c998dd..38ea4e5e7 100644 --- a/app/templates/partials/jobs/count.html +++ b/app/templates/partials/jobs/count.html @@ -2,9 +2,9 @@
diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index b011f88ac..cb87d6280 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -2,9 +2,9 @@
diff --git a/app/templates/partials/jobs/status.html b/app/templates/partials/jobs/status.html index dff1ceda1..2ef0016c7 100644 --- a/app/templates/partials/jobs/status.html +++ b/app/templates/partials/jobs/status.html @@ -1,8 +1,8 @@
diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index 1109249f9..d13cfe2bc 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -4,10 +4,10 @@ {% from "components/table.html" import list_table, field, right_aligned_field_heading, hidden_field_heading %}