Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent polling requests from being queued up #4980

Merged
merged 23 commits into from
Aug 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8d51660
Do not poll until previous requests have been received
alonsogarciapablo Aug 10, 2015
c928303
Revert to using an interval with a boolean flag to determine if fetch…
alonsogarciapablo Aug 10, 2015
316c42c
Typo
alonsogarciapablo Aug 10, 2015
106c085
Extracted Poller class
alonsogarciapablo Aug 10, 2015
22f071a
Extracted tests for Poller
alonsogarciapablo Aug 11, 2015
bdf95d0
Added autoStart option to the Poller
alonsogarciapablo Aug 11, 2015
cce5ac4
Poller needs to be instantiated before polling can actually happen
alonsogarciapablo Aug 11, 2015
2c4868e
Merge remote-tracking branch 'origin/master' into 4939-better-polling
alonsogarciapablo Aug 11, 2015
356141b
Poller accepts a function to specify a variable interval
alonsogarciapablo Aug 11, 2015
4be9e0a
ImportModel uses Poller to poll the model
alonsogarciapablo Aug 11, 2015
5074c0e
Renamed instance variable
alonsogarciapablo Aug 11, 2015
8f10696
`condition` shouldn't determine when to incremente the number of requ…
alonsogarciapablo Aug 11, 2015
ce9aa71
Tests
alonsogarciapablo Aug 11, 2015
0066cb1
Fix test
alonsogarciapablo Aug 11, 2015
5d064c2
Fixed false positives and try to fix jasmine timeout
alonsogarciapablo Aug 11, 2015
c2d18b3
Created a GeocodingModelPoller and ImportModelPoller with specific co…
alonsogarciapablo Aug 12, 2015
e9db1c8
Merge remote-tracking branch 'origin/master' into 4939-better-polling
alonsogarciapablo Aug 12, 2015
ecc6e00
Error callback receives the model as a parameter
alonsogarciapablo Aug 12, 2015
c92bee4
Whitespace
alonsogarciapablo Aug 12, 2015
e23a4a3
Tried to make condition a bit more clear
alonsogarciapablo Aug 12, 2015
e077782
Renamed "condition" option to "stopWhen"
alonsogarciapablo Aug 12, 2015
22d8980
Fixed test
alonsogarciapablo Aug 12, 2015
c9406db
Merge remote-tracking branch 'origin/master' into 4939-better-polling
alonsogarciapablo Aug 12, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
var Backbone = require('backbone');
var _ = require('underscore');
var pollTimer = 2000;
var GeocodingModelPoller = require('./geocoding_model_poller');

/**
* Geocoding model
*
*/

module.exports = cdb.core.Model.extend({

options: {
Expand Down Expand Up @@ -35,16 +34,17 @@ module.exports = cdb.core.Model.extend({
},

initialize: function(opts) {
var self = this;
this._initBinds();
_.extend(this.options, opts);
this.poller = new GeocodingModelPoller(this);
if (this.options.startPollingAutomatically) {
this._checkModel();
}
this._checkModel();
};
},

_initBinds: function() {
this.bind('change:id', this._checkModel, this);
this.bind('change:state', this._checkState, this);
},

_checkModel: function() {
Expand Down Expand Up @@ -74,33 +74,12 @@ module.exports = cdb.core.Model.extend({
}
},

// checks for poll to finish
pollCheck: function(i) {
var self = this;

if (this.pollTimer) {
return false;
}

this.pollTimer = setInterval(function() {
self.fetch({
error: function(e) {
self.trigger("change");
}
});
}, pollTimer);
pollCheck: function() {
this.poller.start();
},

destroyCheck: function() {
clearInterval(this.pollTimer);
delete this.pollTimer;
},

_checkState: function() {
var state = this.get('state');
if (this.hasFailed() || this.hasCompleted()) {
this.destroyCheck();
}
this.poller.stop();
},

getError: function() {
Expand All @@ -121,13 +100,10 @@ module.exports = cdb.core.Model.extend({
},

cancelGeocoding: function() {
this.destroyCheck();
this.save({ state: 'cancelled' }, { wait:true });
},

resetGeocoding: function() {
this.destroyCheck();
this.set('state', 'reset');
}

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
var Poller = require('./poller');

var GeocodingModelPoller = function(model) {

var POLLING_INTERVAL = 2000;

var options = {
interval: POLLING_INTERVAL,
stopWhen: function(model) {
return model.hasFailed() || model.hasCompleted();
},
error: function(model) {
model.trigger("change");
}
};

Poller.call(this, model, options);
};

GeocodingModelPoller.prototype = _.extend({}, Poller.prototype);

module.exports = GeocodingModelPoller;
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
var cdb = require('cartodb.js');
var cdbAdmin = require('cdb.admin');
var pollTimer = 2000; // Interval time between poll checkings
var timerMultiply = 2.5; // Multiply interval for this number
var maxTries = 30; // Max tries until interval change
var ImportModelPoller = require('./import_model_poller');

/**
* New import model that controls
* the state of an import
*
*/

module.exports = cdb.core.Model.extend({

url: function(method) {
Expand All @@ -25,11 +22,10 @@ module.exports = cdb.core.Model.extend({

initialize: function() {
this._initBinds();
this.poller = new ImportModelPoller(this);
},

_initBinds: function() {
this.bind('change:state', this._checkState, this);
this.bind('change:success', this._checkState, this);
this.bind('change:item_queue_id', this._checkQueueId, this);
},

Expand All @@ -39,13 +35,6 @@ module.exports = cdb.core.Model.extend({
this[ data.interval > 0 ? '_createSyncImport' : '_createRegularImport'](d);
},

_checkState: function() {
var state = this.get('state');
if (state === "complete" || state === "failure") {
this.destroyCheck();
}
},

_checkQueueId: function() {
if (this.get('item_queue_id')) {
this.pollCheck();
Expand Down Expand Up @@ -138,28 +127,11 @@ module.exports = cdb.core.Model.extend({
},

pollCheck: function() {
if (this.pollTimer) return;
var self = this;
var tries = 0;
this.pollTimer = setInterval(request, pollTimer);

function request() {
self.destroyCheck();
self.fetch();
++tries;
// Multiply polling timer by a number when a max
// of tries have been reached
var multiply = tries > maxTries ? timerMultiply : 1 ;
self.pollTimer = setInterval(request, pollTimer * multiply);
}

// Start doing a fetch
self.fetch();
this.poller.start();
},

destroyCheck: function() {
clearInterval(this.pollTimer);
delete this.pollTimer;
this.poller.stop();
}

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
var Poller = require('./poller');

var ImportModelPoller = function(model) {

var POLLING_INTERVAL = 2000; // Interval time between poll checkings
var POLLING_INTERVAL_MULTIPLIER = 2.5; // Multiply interval by this number
var POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE = 30; // Max tries until interval change

var options = {
interval: function(numberOfRequests) {
if (numberOfRequests >= POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE) {
return POLLING_INTERVAL * POLLING_INTERVAL_MULTIPLIER;
}
return POLLING_INTERVAL;
},
stopWhen: function(model) {
var state = model.get('state');
return (state === "complete" || state === "failure");
}
};

Poller.call(this, model, options);
};

ImportModelPoller.prototype = _.extend({}, Poller.prototype);

module.exports = ImportModelPoller;
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Periodically fetches a model/collection. It waits for ongoing
* fetch requests before trying to fetch again. A stop condition
* can be specified.
*
* Usage example:
*
* var poller = new Poller(model, {
* interval: 1000,
* stopWhen: function(model) {
* return model.get('state') === 'completed';
* }
* });
*
* poller.start();
*
* // ...
*
* poller.stop();
*
*/
var Poller = function(model, options) {
this.model = model;
this.numberOfRequests = 0;
this.polling = false;
this.interval = options['interval'];
if (typeof this.interval !== "function") {
this.interval = function() { return options['interval']; };
}
this.stopWhen = options['stopWhen'];
this.error = options['error'];
this.autoStart = options['autoStart'];

if (this.autoStart) {
this.start();
}
}

Poller.prototype.start = function() {
if (this.timeout) {
return;
}

this._scheduleFetch();
}

Poller.prototype._scheduleFetch = function() {
this.timeout = setTimeout(this._fetch.bind(this), this.interval(this.numberOfRequests));
}

Poller.prototype._fetch = function() {
var self = this;
if (!self.polling) {
self.polling = true;
self.model.fetch({
success: function() {
self.polling = false;
self.numberOfRequests++;
if (self._continuePolling()) {
self._scheduleFetch();
}
},
error: function(e) {
_.isFunction(self.error) && self.error(self.model);
}
})
}
}

Poller.prototype._continuePolling = function() {
return !this.stopWhen ||
(_.isFunction(this.stopWhen) && !this.stopWhen(this.model));
}

Poller.prototype.stop = function() {
this.polling = false;
clearTimeout(this.timeout);
delete(this.timeout);
}

module.exports = Poller;
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,46 @@ describe('common/background_polling/geocoding_model', function() {
it('should have several change binds', function() {
this.model._initBinds();
expect(this.model.bind.calls.argsFor(0)[0]).toEqual('change:id');
expect(this.model.bind.calls.argsFor(1)[0]).toEqual('change:state');
});

describe('polling', function() {
describe('.pollCheck', function() {

beforeEach(function() {
this.model.set('id', '1');
spyOn(this.model, 'fetch').and.callThrough();
jasmine.clock().install();
spyOn(this.model, 'fetch');
});

afterEach(function() {
jasmine.clock().uninstall();
});

it('should start polling', function() {
spyOn(this.model.poller, 'start');

this.model.pollCheck();

expect(this.model.poller.start).toHaveBeenCalled();
});
});

describe('.destroyCheck', function() {

beforeEach(function() {
jasmine.clock().install();
spyOn(this.model, 'fetch');
});

it('should stop polling when geocoding has failed', function(done) {
var self = this;
setTimeout(function(){
self.model.set('state', 'failed');
expect(self.model.fetch.calls.count()).toBe(1);
expect(self.model.pollTimer).toBeUndefined();
done();
}, 2000);
afterEach(function() {
jasmine.clock().uninstall();
});

it('should stop polling when geocoding has completed', function(done) {
var self = this;
setTimeout(function(){
self.model.set('state', 'finished');
expect(self.model.fetch.calls.count()).toBe(1);
expect(self.model.pollTimer).toBeUndefined();
done();
}, 2000);
it('should stop polling', function() {
spyOn(this.model.poller, 'stop');

this.model.pollCheck();
this.model.destroyCheck();

expect(this.model.poller.stop).toHaveBeenCalled();
});
});

Expand Down
Loading