Commit df629173 authored by Fatih Acet's avatar Fatih Acet

Merge branch '24807-stop-ddosing-ourselves' into 'master'

Stop DDOSing ourselves

## What does this MR do?

Adds `hiddenInterval` to `SmartInterval` settings. This will be the interval used whilst the tab is inactive. If this setting is not set it will `cancel` as it would have done before.

Adds `immediateExecution` to `SmartInterval` settings. This boolean will dictate whether to execute the callback once before the first interval or not.

Uses `SmartInterval` with the new `hiddenInterval` settings to slow the polling for the MR widget on inactive tabs.

Fixes issue where `SmartInterval`s `visibilitychange` listeners are not getting called because jQuery doesn't support them.

## Are there points in the code the reviewer needs to double check?

## Why was this MR needed?

We're ddosing ourselves on the MR page.

## Screenshots (if relevant)

![Screen_Shot_2016-11-25_at_18.36.25](/uploads/c4457c55872f592e921a50cf5462022e/Screen_Shot_2016-11-25_at_18.36.25.png)

~30 seconds difference between the first 2 requests _(a couple requests had fired already so it's backed off already)_.

_-changed to different tab-_

~2 minutes difference between the middle 2 requests.

_-changed tab back to MR-_

~10 seconds difference between the last 2 requests.

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?


Closes #24807

See merge request !7762
parents 7e9a8bb7 62f8717c
......@@ -40,19 +40,26 @@
$('#modal_merge_info').modal({
show: false
});
this.firstCICheck = true;
this.readyForCICheck = false;
this.readyForCIEnvironmentCheck = false;
this.cancel = false;
clearInterval(this.fetchBuildStatusInterval);
clearInterval(this.fetchBuildEnvironmentStatusInterval);
this.clearEventListeners();
this.addEventListeners();
this.getCIStatus(false);
this.getCIEnvironmentsStatus();
this.retrieveSuccessIcon();
this.pollCIStatus();
this.pollCIEnvironmentsStatus();
this.ciStatusInterval = new global.SmartInterval({
callback: this.getCIStatus.bind(this, true),
startingInterval: 10000,
maxInterval: 30000,
hiddenInterval: 120000,
incrementByFactorOf: 5000,
});
this.ciEnvironmentStatusInterval = new global.SmartInterval({
callback: this.getCIEnvironmentsStatus.bind(this),
startingInterval: 30000,
maxInterval: 120000,
hiddenInterval: 240000,
incrementByFactorOf: 15000,
immediateExecution: true,
});
notifyPermissions();
}
......@@ -60,10 +67,6 @@
return $(document).off('page:change.merge_request');
};
MergeRequestWidget.prototype.cancelPolling = function() {
return this.cancel = true;
};
MergeRequestWidget.prototype.addEventListeners = function() {
var allowedPages;
allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes'];
......@@ -72,9 +75,6 @@
var page;
page = $('body').data('page').split(':').last();
if (allowedPages.indexOf(page) < 0) {
clearInterval(_this.fetchBuildStatusInterval);
clearInterval(_this.fetchBuildEnvironmentStatusInterval);
_this.cancelPolling();
return _this.clearEventListeners();
}
};
......@@ -114,6 +114,11 @@
});
};
MergeRequestWidget.prototype.cancelPolling = function () {
this.ciStatusInterval.cancel();
this.ciEnvironmentStatusInterval.cancel();
};
MergeRequestWidget.prototype.getMergeStatus = function() {
return $.get(this.opts.merge_check_url, function(data) {
return $('.mr-state-widget').replaceWith(data);
......@@ -131,18 +136,6 @@
}
};
MergeRequestWidget.prototype.pollCIStatus = function() {
return this.fetchBuildStatusInterval = setInterval(((function(_this) {
return function() {
if (!_this.readyForCICheck) {
return;
}
_this.getCIStatus(true);
return _this.readyForCICheck = false;
};
})(this)), 10000);
};
MergeRequestWidget.prototype.getCIStatus = function(showNotification) {
var _this;
_this = this;
......@@ -150,23 +143,17 @@
return $.getJSON(this.opts.ci_status_url, (function(_this) {
return function(data) {
var message, status, title;
if (_this.cancel) {
return;
}
_this.readyForCICheck = true;
if (data.status === '') {
return;
}
if (data.environments && data.environments.length) _this.renderEnvironments(data.environments);
if (_this.firstCICheck || data.status !== _this.opts.ci_status && (data.status != null)) {
if (data.status !== _this.opts.ci_status && (data.status != null)) {
_this.opts.ci_status = data.status;
_this.showCIStatus(data.status);
if (data.coverage) {
_this.showCICoverage(data.coverage);
}
// The first check should only update the UI, a notification
// should only be displayed on status changes
if (showNotification && !_this.firstCICheck) {
if (showNotification) {
status = _this.ciLabelForStatus(data.status);
if (status === "preparing") {
title = _this.opts.ci_title.preparing;
......@@ -184,24 +171,13 @@
return Turbolinks.visit(_this.opts.builds_path);
});
}
return _this.firstCICheck = false;
}
};
})(this));
};
MergeRequestWidget.prototype.pollCIEnvironmentsStatus = function() {
this.fetchBuildEnvironmentStatusInterval = setInterval(() => {
if (!this.readyForCIEnvironmentCheck) return;
this.getCIEnvironmentsStatus();
this.readyForCIEnvironmentCheck = false;
}, 300000);
};
MergeRequestWidget.prototype.getCIEnvironmentsStatus = function() {
$.getJSON(this.opts.ci_environments_status_url, (environments) => {
if (this.cancel) return;
this.readyForCIEnvironmentCheck = true;
if (environments && environments.length) this.renderEnvironments(environments);
});
};
......
......@@ -7,24 +7,31 @@
(() => {
class SmartInterval {
/**
* @param { function } callback Function to be called on each iteration (required)
* @param { milliseconds } startingInterval `currentInterval` is set to this initially
* @param { milliseconds } maxInterval `currentInterval` will be incremented to this
* @param { integer } incrementByFactorOf `currentInterval` is incremented by this factor
* @param { boolean } lazyStart Configure if timer is initialized on instantiation or lazily
* @param { function } opts.callback Function to be called on each iteration (required)
* @param { milliseconds } opts.startingInterval `currentInterval` is set to this initially
* @param { milliseconds } opts.maxInterval `currentInterval` will be incremented to this
* @param { milliseconds } opts.hiddenInterval `currentInterval` is set to this
* when the page is hidden
* @param { integer } opts.incrementByFactorOf `currentInterval` is incremented by this factor
* @param { boolean } opts.lazyStart Configure if timer is initialized on
* instantiation or lazily
* @param { boolean } opts.immediateExecution Configure if callback should
* be executed before the first interval.
*/
constructor({ callback, startingInterval, maxInterval, incrementByFactorOf, lazyStart }) {
constructor(opts = {}) {
this.cfg = {
callback,
startingInterval,
maxInterval,
incrementByFactorOf,
lazyStart,
callback: opts.callback,
startingInterval: opts.startingInterval,
maxInterval: opts.maxInterval,
hiddenInterval: opts.hiddenInterval,
incrementByFactorOf: opts.incrementByFactorOf,
lazyStart: opts.lazyStart,
immediateExecution: opts.immediateExecution,
};
this.state = {
intervalId: null,
currentInterval: startingInterval,
currentInterval: this.cfg.startingInterval,
pageVisibility: 'visible',
};
......@@ -36,6 +43,11 @@
const cfg = this.cfg;
const state = this.state;
if (cfg.immediateExecution) {
cfg.immediateExecution = false;
cfg.callback();
}
state.intervalId = window.setInterval(() => {
cfg.callback();
......@@ -54,14 +66,29 @@
this.stopTimer();
}
onVisibilityHidden() {
if (this.cfg.hiddenInterval) {
this.setCurrentInterval(this.cfg.hiddenInterval);
this.resume();
} else {
this.cancel();
}
}
// start a timer, using the existing interval
resume() {
this.stopTimer(); // stop exsiting timer, in case timer was not previously stopped
this.start();
}
onVisibilityVisible() {
this.cancel();
this.start();
}
destroy() {
this.cancel();
document.removeEventListener('visibilitychange', this.handleVisibilityChange);
$(document).off('visibilitychange').off('page:before-unload');
}
......@@ -80,11 +107,7 @@
initVisibilityChangeHandling() {
// cancel interval when tab no longer shown (prevents cached pages from polling)
$(document)
.off('visibilitychange').on('visibilitychange', (e) => {
this.state.pageVisibility = e.target.visibilityState;
this.handleVisibilityChange();
});
document.addEventListener('visibilitychange', this.handleVisibilityChange.bind(this));
}
initPageUnloadHandling() {
......@@ -92,10 +115,11 @@
$(document).on('page:before-unload', () => this.cancel());
}
handleVisibilityChange() {
const state = this.state;
const intervalAction = state.pageVisibility === 'hidden' ? this.cancel : this.resume;
handleVisibilityChange(e) {
this.state.pageVisibility = e.target.visibilityState;
const intervalAction = this.isPageVisible() ?
this.onVisibilityVisible :
this.onVisibilityHidden;
intervalAction.apply(this);
}
......@@ -111,6 +135,7 @@
incrementInterval() {
const cfg = this.cfg;
const currentInterval = this.getCurrentInterval();
if (cfg.hiddenInterval && !this.isPageVisible()) return;
let nextInterval = currentInterval * cfg.incrementByFactorOf;
if (nextInterval > cfg.maxInterval) {
......@@ -120,6 +145,8 @@
this.setCurrentInterval(nextInterval);
}
isPageVisible() { return this.state.pageVisibility === 'visible'; }
stopTimer() {
const state = this.state;
......
---
title: Use SmartInterval for MR widget and improve visibilitychange functionality
merge_request: 7762
author:
......@@ -14,8 +14,9 @@
startingInterval: DEFAULT_STARTING_INTERVAL,
maxInterval: DEFAULT_MAX_INTERVAL,
incrementByFactorOf: DEFAULT_INCREMENT_FACTOR,
delayStartBy: 0,
lazyStart: false,
immediateExecution: false,
hiddenInterval: null,
};
if (config) {
......@@ -114,14 +115,31 @@
expect(interval.state.intervalId).toBeTruthy();
// simulates triggering of visibilitychange event
interval.state.pageVisibility = 'hidden';
interval.handleVisibilityChange();
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeUndefined();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should change to the hidden interval when page is not visible', function (done) {
const HIDDEN_INTERVAL = 1500;
const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should resume when page is becomes visible at the previous interval', function (done) {
const interval = this.smartInterval;
......@@ -129,14 +147,12 @@
expect(interval.state.intervalId).toBeTruthy();
// simulates triggering of visibilitychange event
interval.state.pageVisibility = 'hidden';
interval.handleVisibilityChange();
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeUndefined();
// simulates triggering of visibilitychange event
interval.state.pageVisibility = 'visible';
interval.handleVisibilityChange();
interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
expect(interval.state.intervalId).toBeTruthy();
......@@ -154,6 +170,11 @@
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should execute callback before first interval', function () {
const interval = createDefaultSmartInterval({ immediateExecution: true });
expect(interval.cfg.immediateExecution).toBeFalsy();
});
});
});
})(window.gl || (window.gl = {}));
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment