Skip to content

Commit 76249cb

Browse files
committed
refactor: use common methods for duration calculations
We use `process.hrtime` a lot throughout the codebase to track durations for certain operations, often storing the result of it on an object and using it later for duration calculation. This patch changes that to depend on numbers rather than hrtime, which would make it easy in the future to use something like `performance.now` or even degrade into `Date.now` in environments which don't support hi-res timers. NODE-2651
1 parent 6f3048c commit 76249cb

File tree

11 files changed

+54
-40
lines changed

11 files changed

+54
-40
lines changed

lib/change_stream.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ const Cursor = require('./cursor');
88
const relayEvents = require('./core/utils').relayEvents;
99
const maxWireVersion = require('./core/utils').maxWireVersion;
1010
const maybePromise = require('./utils').maybePromise;
11+
const now = require('./utils').now;
12+
const calculateDurationInMs = require('./utils').calculateDurationInMs;
1113
const AggregateOperation = require('./operations/aggregate');
1214

1315
const kResumeQueue = Symbol('resumeQueue');
@@ -459,15 +461,21 @@ function applyKnownOptions(target, source, optionNames) {
459461
const SELECTION_TIMEOUT = 30000;
460462
function waitForTopologyConnected(topology, options, callback) {
461463
setTimeout(() => {
462-
if (options && options.start == null) options.start = process.hrtime();
463-
const start = options.start || process.hrtime();
464+
if (options && options.start == null) {
465+
options.start = now();
466+
}
467+
468+
const start = options.start || now();
464469
const timeout = options.timeout || SELECTION_TIMEOUT;
465470
const readPreference = options.readPreference;
471+
if (topology.isConnected({ readPreference })) {
472+
return callback();
473+
}
474+
475+
if (calculateDurationInMs(start) > timeout) {
476+
return callback(new MongoError('Timed out waiting for connection'));
477+
}
466478

467-
if (topology.isConnected({ readPreference })) return callback();
468-
const hrElapsed = process.hrtime(start);
469-
const elapsed = (hrElapsed[0] * 1e9 + hrElapsed[1]) / 1e6;
470-
if (elapsed > timeout) return callback(new MongoError('Timed out waiting for connection'));
471479
waitForTopologyConnected(topology, options, callback);
472480
}, 500); // this is an arbitrary wait time to allow SDAM to transition
473481
}

lib/cmap/connection.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const wp = require('../core/wireprotocol');
1111
const apm = require('../core/connection/apm');
1212
const updateSessionFromResponse = require('../core/sessions').updateSessionFromResponse;
1313
const uuidV4 = require('../core/utils').uuidV4;
14+
const now = require('../utils').now;
15+
const calculateDurationInMs = require('../utils').calculateDurationInMs;
1416

1517
const kStream = Symbol('stream');
1618
const kQueue = Symbol('queue');
@@ -37,7 +39,7 @@ class Connection extends EventEmitter {
3739

3840
this[kDescription] = new StreamDescription(this.address, options);
3941
this[kGeneration] = options.generation;
40-
this[kLastUseTime] = Date.now();
42+
this[kLastUseTime] = now();
4143

4244
// retain a reference to an `AutoEncrypter` if present
4345
if (options.autoEncrypter) {
@@ -108,7 +110,7 @@ class Connection extends EventEmitter {
108110
}
109111

110112
get idleTime() {
111-
return Date.now() - this[kLastUseTime];
113+
return calculateDurationInMs(this[kLastUseTime]);
112114
}
113115

114116
get clusterTime() {
@@ -120,7 +122,7 @@ class Connection extends EventEmitter {
120122
}
121123

122124
markAvailable() {
123-
this[kLastUseTime] = Date.now();
125+
this[kLastUseTime] = now();
124126
}
125127

126128
destroy(options, callback) {
@@ -326,7 +328,7 @@ function write(command, options, callback) {
326328
if (this.monitorCommands) {
327329
this.emit('commandStarted', new apm.CommandStartedEvent(this, command));
328330

329-
operationDescription.started = process.hrtime();
331+
operationDescription.started = now();
330332
operationDescription.cb = (err, reply) => {
331333
if (err) {
332334
this.emit(

lib/core/connection/apm.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
const Msg = require('../connection/msg').Msg;
33
const KillCursor = require('../connection/commands').KillCursor;
44
const GetMore = require('../connection/commands').GetMore;
5-
const calculateDurationInMs = require('../utils').calculateDurationInMs;
5+
const calculateDurationInMs = require('../../utils').calculateDurationInMs;
66

77
/** Commands that we want to redact because of the sensitive nature of their contents */
88
const SENSITIVE_COMMANDS = new Set([

lib/core/connection/pool.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const connect = require('./connect');
2121
const updateSessionFromResponse = require('../sessions').updateSessionFromResponse;
2222
const eachAsync = require('../utils').eachAsync;
2323
const makeStateMachine = require('../utils').makeStateMachine;
24+
const now = require('../../utils').now;
2425

2526
const DISCONNECTED = 'disconnected';
2627
const CONNECTING = 'connecting';
@@ -898,7 +899,7 @@ Pool.prototype.write = function(command, options, cb) {
898899
if (self.options.monitorCommands) {
899900
this.emit('commandStarted', new apm.CommandStartedEvent(this, command));
900901

901-
operation.started = process.hrtime();
902+
operation.started = now();
902903
operation.cb = (err, reply) => {
903904
if (err) {
904905
self.emit(

lib/core/sdam/monitor.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
'use strict';
22

33
const ServerType = require('./common').ServerType;
4-
const calculateDurationInMs = require('../utils').calculateDurationInMs;
54
const EventEmitter = require('events');
65
const connect = require('../connection/connect');
76
const Connection = require('../../cmap/connection').Connection;
87
const common = require('./common');
98
const makeStateMachine = require('../utils').makeStateMachine;
109
const MongoError = require('../error').MongoError;
1110
const makeInterruptableAsyncInterval = require('../../utils').makeInterruptableAsyncInterval;
11+
const calculateDurationInMs = require('../../utils').calculateDurationInMs;
12+
const now = require('../../utils').now;
1213

1314
const sdamEvents = require('./events');
1415
const ServerHeartbeatStartedEvent = sdamEvents.ServerHeartbeatStartedEvent;
@@ -138,7 +139,7 @@ function checkServer(monitor, callback) {
138139
monitor[kConnection] = undefined;
139140
}
140141

141-
const start = process.hrtime();
142+
const start = now();
142143
monitor.emit('serverHeartbeatStarted', new ServerHeartbeatStartedEvent(monitor.address));
143144

144145
function failureHandler(err) {

lib/core/sdam/server_description.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const arrayStrictEqual = require('../utils').arrayStrictEqual;
44
const tagsStrictEqual = require('../utils').tagsStrictEqual;
55
const errorStrictEqual = require('../utils').errorStrictEqual;
66
const ServerType = require('./common').ServerType;
7+
const now = require('../../utils').now;
78

89
const WRITABLE_SERVER_TYPES = new Set([
910
ServerType.RSPrimary,
@@ -70,7 +71,7 @@ class ServerDescription {
7071
this.address = address;
7172
this.error = options.error;
7273
this.roundTripTime = options.roundTripTime || -1;
73-
this.lastUpdateTime = Date.now();
74+
this.lastUpdateTime = now();
7475
this.lastWriteDate = ismaster.lastWrite ? ismaster.lastWrite.lastWriteDate : null;
7576
this.opTime = ismaster.lastWrite ? ismaster.lastWrite.opTime : null;
7677
this.type = parseServerType(ismaster);

lib/core/sessions.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ const isTransactionCommand = require('./transactions').isTransactionCommand;
1717
const resolveClusterTime = require('./topologies/shared').resolveClusterTime;
1818
const isSharded = require('./wireprotocol/shared').isSharded;
1919
const maxWireVersion = require('./utils').maxWireVersion;
20-
20+
const now = require('./../utils').now;
21+
const calculateDurationInMs = require('./../utils').calculateDurationInMs;
2122
const minWireVersionForShardedTransactions = 8;
2223

2324
function assertAlive(session, callback) {
@@ -285,7 +286,7 @@ class ClientSession extends EventEmitter {
285286
* @param {TransactionOptions} [options] Optional settings for the transaction
286287
*/
287288
withTransaction(fn, options) {
288-
const startTime = Date.now();
289+
const startTime = now();
289290
return attemptTransaction(this, startTime, fn, options);
290291
}
291292
}
@@ -301,7 +302,7 @@ const NON_DETERMINISTIC_WRITE_CONCERN_ERRORS = new Set([
301302
]);
302303

303304
function hasNotTimedOut(startTime, max) {
304-
return Date.now() - startTime < max;
305+
return calculateDurationInMs(startTime) < max;
305306
}
306307

307308
function isUnknownTransactionCommitResult(err) {
@@ -558,7 +559,7 @@ function supportsRecoveryToken(session) {
558559
class ServerSession {
559560
constructor() {
560561
this.id = { id: new Binary(uuidV4(), Binary.SUBTYPE_UUID) };
561-
this.lastUse = Date.now();
562+
this.lastUse = now();
562563
this.txnNumber = 0;
563564
this.isDirty = false;
564565
}
@@ -573,7 +574,7 @@ class ServerSession {
573574
// Take the difference of the lastUse timestamp and now, which will result in a value in
574575
// milliseconds, and then convert milliseconds to minutes to compare to `sessionTimeoutMinutes`
575576
const idleTimeMinutes = Math.round(
576-
(((Date.now() - this.lastUse) % 86400000) % 3600000) / 60000
577+
((calculateDurationInMs(this.lastUse) % 86400000) % 3600000) / 60000
577578
);
578579

579580
return idleTimeMinutes > sessionTimeoutMinutes - 1;
@@ -708,7 +709,7 @@ function applySession(session, command, options) {
708709
}
709710

710711
const serverSession = session.serverSession;
711-
serverSession.lastUse = Date.now();
712+
serverSession.lastUse = now();
712713
command.lsid = serverSession.id;
713714

714715
// first apply non-transaction-specific sessions data

lib/core/topologies/replset.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ const isRetryableWritesSupported = require('./shared').isRetryableWritesSupporte
1717
const relayEvents = require('../utils').relayEvents;
1818
const isRetryableError = require('../error').isRetryableError;
1919
const BSON = retrieveBSON();
20-
const calculateDurationInMs = require('../utils').calculateDurationInMs;
2120
const getMMAPError = require('./shared').getMMAPError;
2221
const makeClientMetadata = require('../utils').makeClientMetadata;
22+
const now = require('../../utils').now;
23+
const calculateDurationInMs = require('../../utils').calculateDurationInMs;
2324

2425
//
2526
// States
@@ -426,8 +427,7 @@ var pingServer = function(self, server, cb) {
426427
var latencyMS = new Date().getTime() - start;
427428

428429
// Set the last updatedTime
429-
var hrtime = process.hrtime();
430-
server.lastUpdateTime = (hrtime[0] * 1e9 + hrtime[1]) / 1e6;
430+
server.lastUpdateTime = now();
431431

432432
// We had an error, remove it from the state
433433
if (err) {
@@ -1127,7 +1127,7 @@ ReplSet.prototype.selectServer = function(selector, options, callback) {
11271127
}
11281128

11291129
let lastError;
1130-
const start = process.hrtime();
1130+
const start = now();
11311131
const _selectServer = () => {
11321132
if (calculateDurationInMs(start) >= SERVER_SELECTION_TIMEOUT_MS) {
11331133
if (lastError != null) {

lib/core/utils.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,6 @@ const uuidV4 = () => {
1313
return result;
1414
};
1515

16-
/**
17-
* Returns the duration calculated from two high resolution timers in milliseconds
18-
*
19-
* @param {Object} started A high resolution timestamp created from `process.hrtime()`
20-
* @returns {Number} The duration in milliseconds
21-
*/
22-
const calculateDurationInMs = started => {
23-
const hrtime = process.hrtime(started);
24-
return (hrtime[0] * 1e9 + hrtime[1]) / 1e6;
25-
};
26-
2716
/**
2817
* Relays events for a given listener and emitter
2918
*
@@ -261,7 +250,6 @@ const noop = () => {};
261250

262251
module.exports = {
263252
uuidV4,
264-
calculateDurationInMs,
265253
relayEvents,
266254
collationNotSupported,
267255
retrieveEJSON,

lib/utils.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,16 @@ function now() {
739739
return Math.floor(hrtime[0] * 1000 + hrtime[1] / 1000000);
740740
}
741741

742+
function calculateDurationInMs(started) {
743+
if (typeof started !== 'number') {
744+
console.trace('HERE');
745+
throw TypeError('numeric value required to calculate duration');
746+
}
747+
748+
const elapsed = now() - started;
749+
return elapsed < 0 ? 0 : elapsed;
750+
}
751+
742752
/**
743753
* Creates an interval timer which is able to be woken up sooner than
744754
* the interval. The timer will also debounce multiple calls to wake
@@ -849,5 +859,6 @@ module.exports = {
849859
makeCounter,
850860
maybePromise,
851861
now,
862+
calculateDurationInMs,
852863
makeInterruptableAsyncInterval
853864
};

test/unit/core/sessions.test.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const mock = require('mongodb-mock-server');
44
const expect = require('chai').expect;
55
const genClusterTime = require('./common').genClusterTime;
66
const sessionCleanupHandler = require('./common').sessionCleanupHandler;
7+
const now = require('../../../lib/utils').now;
78

89
const core = require('../../../lib/core');
910
const Server = core.Server;
@@ -138,7 +139,7 @@ describe('Sessions', function() {
138139

139140
test: function(done) {
140141
const oldSession = new ServerSession();
141-
oldSession.lastUse = new Date(Date.now() - 30 * 60 * 1000).getTime(); // add 30min
142+
oldSession.lastUse = now() - 30 * 60 * 1000; // add 30min
142143

143144
const pool = new ServerSessionPool(test.client);
144145
done = sessionCleanupHandler(null, pool, done);
@@ -159,7 +160,7 @@ describe('Sessions', function() {
159160
test: function(done) {
160161
const newSession = new ServerSession();
161162
const oldSessions = [new ServerSession(), new ServerSession()].map(session => {
162-
session.lastUse = new Date(Date.now() - 30 * 60 * 1000).getTime(); // add 30min
163+
session.lastUse = now() - 30 * 60 * 1000; // add 30min
163164
return session;
164165
});
165166

@@ -179,7 +180,7 @@ describe('Sessions', function() {
179180

180181
test: function(done) {
181182
const session = new ServerSession();
182-
session.lastUse = new Date(Date.now() - 9.5 * 60 * 1000).getTime(); // add 9.5min
183+
session.lastUse = now() - 9.5 * 60 * 1000; // add 9.5min
183184

184185
const pool = new ServerSessionPool(test.client);
185186
done = sessionCleanupHandler(null, pool, done);

0 commit comments

Comments
 (0)