Skip to content

Commit

Permalink
DDP TimeSync + Tests
Browse files Browse the repository at this point in the history
- Added _timeSync Meteor Method for doing timesync over DDP instead of HTTP
- Auto switch to DDP after initial HTTP timesync to improve subsequent round trip times
- Added option TimeSync.forceDDP to always use DDP, even for first sync (which may be slow!)
- Shortened resync interval from 1 minute to 30 seconds when using DDP.
- Added tests for DDP and HTTP sync
- Added option to set the timesync URL using `TimeSync.setSyncUrl`
  • Loading branch information
wreiske committed Mar 19, 2019
1 parent af96690 commit cb9f157
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 37 deletions.
9 changes: 8 additions & 1 deletion History.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
- Code Format Refactor
- Bumped minimum package API version to Meteor 1.3 (to support Tracker)
- Changed Deps to Tracker (#49)
- Only show log output in development
- Only show log output if running in development
- Added _timeSync Meteor Method for doing timesync over DDP instead of HTTP
- Auto switch to DDP after initial HTTP timesync to improve subsequent round trip times
- Added option TimeSync.forceDDP to always use DDP, even for first sync (which may be slow!)
- Shortened resync interval from 1 minute to 30 seconds when using DDP.
- Added tests for DDP and HTTP sync
- Added option to set the timesync URL using `TimeSync.setSyncUrl`
- ... more to come, v0.6.0 is a work in progress!

## v0.5.1

Expand Down
90 changes: 56 additions & 34 deletions client/timesync-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ Date.now = Date.now || function () {
};

TimeSync = {
loggingEnabled: Meteor.isDevelopment
loggingEnabled: Meteor.isDevelopment,
forceDDP: false
};

function log( /* arguments */ ) {
Expand All @@ -26,6 +27,7 @@ SyncInternals = {
offsetTracker: new Tracker.Dependency(),
syncTracker: new Tracker.Dependency(),
isSynced: false,
usingDDP: false,
timeTick: {},
getDiscrepancy: function (lastTime, currentTime, interval) {
return currentTime - (lastTime + interval)
Expand All @@ -46,44 +48,62 @@ let attempts = 0;
*/

let syncUrl;
if (Meteor.isCordova || Meteor.isDesktop) {
// Only use Meteor.absoluteUrl for Cordova and Desktop; see
// https://github.com/meteor/meteor/issues/4696
// https://github.com/mizzao/meteor-timesync/issues/30
// Cordova should never be running out of a subdirectory...
syncUrl = Meteor.absoluteUrl('_timesync');
} else {
// Support Meteor running in relative paths, based on computed root url prefix
// https://github.com/mizzao/meteor-timesync/pull/40
const basePath = __meteor_runtime_config__.ROOT_URL_PATH_PREFIX || '';
syncUrl = basePath + '/_timesync';

TimeSync.setSyncUrl = function (url) {
if (url) {
syncUrl = url;
} else if (Meteor.isCordova || Meteor.isDesktop) {
// Only use Meteor.absoluteUrl for Cordova and Desktop; see
// https://github.com/meteor/meteor/issues/4696
// https://github.com/mizzao/meteor-timesync/issues/30
// Cordova should never be running out of a subdirectory...
syncUrl = Meteor.absoluteUrl('_timesync');
} else {
// Support Meteor running in relative paths, based on computed root url prefix
// https://github.com/mizzao/meteor-timesync/pull/40
const basePath = __meteor_runtime_config__.ROOT_URL_PATH_PREFIX || '';
syncUrl = basePath + '/_timesync';
}
};
TimeSync.getSyncUrl = function () {
return syncUrl;
}
TimeSync.setSyncUrl();

const updateOffset = function () {
const t0 = Date.now();
if (TimeSync.forceDDP || SyncInternals.useDDP) {
Meteor.call('_timeSync', function (err, res) {

This comment has been minimized.

Copy link
@wreiske

wreiske Mar 19, 2019

Author Collaborator

Need to do more testing with this, maybe even pull it out completely... Meteor methods seem to be very slow! After looking through the commit history, it looks like rawConnectHandlers is the fastest way to go.

handleResponse(t0, err, res);
});
} else {
HTTP.get(syncUrl, function (err, res) {
handleResponse(t0, err, res);
});
}
};

HTTP.get(syncUrl, function (err, response) {
const t3 = Date.now(); // Grab this now
if (err) {
// We'll still use our last computed offset if is defined
log('Error syncing to server time: ', err);
if (++attempts <= maxAttempts) {
Meteor.setTimeout(TimeSync.resync, 1000);
} else {
log('Max number of time sync attempts reached. Giving up.');
}
return;
const handleResponse = function (t0, err, res) {
const t3 = Date.now(); // Grab this now
if (err) {
// We'll still use our last computed offset if is defined
log('Error syncing to server time: ', err);
if (++attempts <= maxAttempts) {
Meteor.setTimeout(TimeSync.resync, 1000);
} else {
log('Max number of time sync attempts reached. Giving up.');
}
return;
}

attempts = 0; // It worked

const ts = parseInt(response.content, 10);
SyncInternals.isSynced = true;
SyncInternals.offset = Math.round(((ts - t0) + (ts - t3)) / 2);
SyncInternals.roundTripTime = t3 - t0; // - (ts - ts) which is 0
SyncInternals.offsetTracker.changed();
});
};
attempts = 0; // It worked
const response = res.content || res;
const ts = parseInt(response, 10);
SyncInternals.isSynced = true;
SyncInternals.offset = Math.round(((ts - t0) + (ts - t3)) / 2);
SyncInternals.roundTripTime = t3 - t0; // - (ts - ts) which is 0
SyncInternals.offsetTracker.changed();
}

// Reactive variable for server time that updates every second.
TimeSync.serverTime = function (clientTime, interval) {
Expand Down Expand Up @@ -116,8 +136,9 @@ let resyncIntervalId = null;

TimeSync.resync = function () {
if (resyncIntervalId !== null) Meteor.clearInterval(resyncIntervalId);

updateOffset();
resyncIntervalId = Meteor.setInterval(updateOffset, 600000);
resyncIntervalId = Meteor.setInterval(updateOffset, (SyncInternals.useDDP) ? 300000 : 600000);
};

// Run this as soon as we load, even before Meteor.startup()
Expand All @@ -128,6 +149,7 @@ Tracker.autorun(function () {
const connected = Meteor.status().connected;
if (connected && !wasConnected) TimeSync.resync();
wasConnected = connected;
SyncInternals.useDDP = connected;
});

// Resync if unexpected change by more than a few seconds. This needs to be
Expand Down Expand Up @@ -158,7 +180,7 @@ function getTickDependency(interval) {
Meteor.setInterval(function () {
const currentClientTime = Date.now();
const discrepancy = SyncInternals.getDiscrepancy(lastClientTime, currentClientTime, defaultInterval);

if (Math.abs(discrepancy) < tickCheckTolerance) {
// No problem here, just keep ticking along
SyncInternals.timeTick[defaultInterval].changed();
Expand Down
2 changes: 1 addition & 1 deletion package.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Package.onTest(function (api) {
'test-helpers'
]);

api.use(['tracker'], 'client');
api.use(['tracker', 'underscore'], 'client');

api.use('mizzao:timesync');

Expand Down
11 changes: 10 additions & 1 deletion server/timesync-server.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Meteor } from "meteor/meteor";

// Use rawConnectHandlers so we get a response as quickly as possible
// https://github.com/meteor/meteor/blob/devel/packages/webapp/webapp_server.js

Expand Down Expand Up @@ -26,4 +28,11 @@ WebApp.rawConnectHandlers.use('/_timesync',

res.end(Date.now().toString());
}
);
);

Meteor.methods({
_timeSync: function () {
this.unblock();
return Date.now();
}
});
40 changes: 40 additions & 0 deletions tests/client.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { Meteor } from 'meteor/meteor';
import { Tracker } from 'meteor/tracker';
import { HTTP } from 'meteor/http';

Tinytest.add("timesync - tick check - normal tick", function (test) {
var lastTime = 5000;
var currentTime = 6000;
Expand Down Expand Up @@ -118,3 +122,39 @@ Tinytest.addAsync("timesync - basic - different sync intervals", function (test,
}, testInterval);

});

Tinytest.addAsync("timesync - basic - DDP timeSync", function (test, next) {
Meteor.call('_timeSync', function (err, res) {
if (err) {
test.fail();
next();
}
test.isTrue(_.isNumber(res));

// Make sure it's close to the current time on the client. This should
// always be true in PhantomJS tests where client/server are the same
// machine, although it might fail in development environments, for example
// when the server and client are different VMs.
test.isTrue(Math.abs(res - Date.now()) < 1000);

next();
});
});

Tinytest.addAsync("timesync - basic - HTTP timeSync", function (test, next) {
var syncUrl = TimeSync.getSyncUrl();

test.isNotNull(syncUrl);

HTTP.get(syncUrl, function (err, res) {
if (err) {
test.fail();
next();
}
test.isTrue(res.content);
var serverTime = parseInt(res.content,10);
test.isTrue(_.isNumber(serverTime));
test.isTrue(Math.abs(serverTime - Date.now()) < 1000);
next();
});
});

0 comments on commit cb9f157

Please sign in to comment.