Skip to content

Commit be86f3a

Browse files
committed
Added ability to enforce max connection lifetime
Driver keeps idle network connections in a connection pool. These connections can get invalidated while resting idle in the pool. They can be killed by network equipment like load balancer, proxy or firewall. It is also safer to refresh connections once in a while so that database can renew corresponding resources. This commit adds `maxConnectionLifetime` setting and makes driver close too old connections. Checking and closing happens during connection acquisition. Also removed manual `Date.now()` mocking in favour of existing library - Lolex.
1 parent 6054949 commit be86f3a

12 files changed

+241
-43
lines changed

src/v1/driver.js

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ class Driver {
4747
* @protected
4848
*/
4949
constructor(url, userAgent, token = {}, config = {}) {
50+
sanitizeConfig(config);
51+
5052
this._url = url;
5153
this._userAgent = userAgent;
5254
this._openSessions = {};
@@ -56,7 +58,7 @@ class Driver {
5658
this._pool = new Pool(
5759
this._createConnection.bind(this),
5860
this._destroyConnection.bind(this),
59-
Driver._validateConnection.bind(this),
61+
this._validateConnection.bind(this),
6062
config.connectionPoolSize
6163
);
6264

@@ -90,8 +92,20 @@ class Driver {
9092
* @return {boolean} true if the connection is open
9193
* @access private
9294
**/
93-
static _validateConnection(conn) {
94-
return conn.isOpen();
95+
_validateConnection(conn) {
96+
if (!conn.isOpen()) {
97+
return false;
98+
}
99+
100+
const maxConnectionLifetime = this._config.maxConnectionLifetime;
101+
if (maxConnectionLifetime) {
102+
const lifetime = Date.now() - conn.creationTimestamp;
103+
if (lifetime > maxConnectionLifetime) {
104+
return false;
105+
}
106+
}
107+
108+
return true;
95109
}
96110

97111
/**
@@ -213,7 +227,22 @@ class _ConnectionStreamObserver extends StreamObserver {
213227
}
214228
}
215229

216-
230+
/**
231+
* @private
232+
*/
233+
function sanitizeConfig(config) {
234+
const maxConnectionLifetime = config.maxConnectionLifetime;
235+
if (maxConnectionLifetime) {
236+
const sanitizedMaxConnectionLifetime = parseInt(maxConnectionLifetime, 10);
237+
if (sanitizedMaxConnectionLifetime && sanitizedMaxConnectionLifetime > 0) {
238+
config.maxConnectionLifetime = sanitizedMaxConnectionLifetime;
239+
} else {
240+
config.maxConnectionLifetime = null;
241+
}
242+
} else {
243+
config.maxConnectionLifetime = null;
244+
}
245+
}
217246

218247
export {Driver, READ, WRITE}
219248

src/v1/index.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ const USER_AGENT = "neo4j-javascript/" + VERSION;
9393
* // TRUST_SYSTEM_CA_SIGNED_CERTIFICATES meand that you trust whatever certificates
9494
* // are in the default certificate chain of th
9595
* trust: "TRUST_ALL_CERTIFICATES" | "TRUST_ON_FIRST_USE" | "TRUST_SIGNED_CERTIFICATES" |
96-
* "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES" | "TRUST_SYSTEM_CA_SIGNED_CERTIFICATES",
96+
* "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES" | "TRUST_SYSTEM_CA_SIGNED_CERTIFICATES",
9797
*
9898
* // List of one or more paths to trusted encryption certificates. This only
9999
* // works in the NodeJS bundle, and only matters if you use "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES".
@@ -112,11 +112,20 @@ const USER_AGENT = "neo4j-javascript/" + VERSION;
112112
* // Connection will be destroyed if this threshold is exceeded.
113113
* connectionPoolSize: 50,
114114
*
115+
* // The maximum allowed lifetime for a pooled connection in milliseconds. Pooled connections older than this
116+
* // threshold will be closed and removed from the pool. Such discarding happens during connection acquisition
117+
* // so that new session is never backed by an old connection. Setting this option to a low value will cause
118+
* // a high connection churn and might result in a performance hit. It is recommended to set maximum lifetime
119+
* // to a slightly smaller value than the one configured in network equipment (load balancer, proxy, firewall,
120+
* // etc. can also limit maximum connection lifetime). No maximum lifetime limit is imposed by default. Zero
121+
* // and negative values result in lifetime not being checked.
122+
* maxConnectionLifetime: 30 * 60 * 1000, // 30 minutes
123+
*
115124
* // Specify the maximum time in milliseconds transactions are allowed to retry via
116125
* // {@link Session#readTransaction()} and {@link Session#writeTransaction()} functions. These functions
117126
* // will retry the given unit of work on `ServiceUnavailable`, `SessionExpired` and transient errors with
118127
* // exponential backoff using initial delay of 1 second. Default value is 30000 which is 30 seconds.
119-
* maxTransactionRetryTime: 30000,
128+
* maxTransactionRetryTime: 30000, // 30 seconds
120129
*
121130
* // Provide an alternative load balancing strategy for the routing driver to use.
122131
* // Driver uses "least_connected" by default.

src/v1/internal/connector.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ class Connection {
174174
*/
175175
this.url = url;
176176
this.server = {address: url};
177+
this.creationTimestamp = Date.now();
177178
this._pendingObservers = [];
178179
this._currentObserver = undefined;
179180
this._ch = channel;

src/v1/internal/transaction-executor.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ export default class TransactionExecutor {
5454
}
5555

5656
_retryTransactionPromise(transactionCreator, transactionWork, error, retryStartTime, retryDelayMs) {
57-
const elapsedTimeMs = Date.now() - retryStartTime;
57+
const now = Date.now();
58+
const elapsedTimeMs = now - retryStartTime;
5859

5960
if (elapsedTimeMs > this._maxRetryTimeMs || !TransactionExecutor._canRetryOn(error)) {
6061
return Promise.reject(error);

test/internal/connector.test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,19 @@ import {alloc} from '../../src/v1/internal/buf';
2525
import {Neo4jError} from '../../src/v1/error';
2626
import sharedNeo4j from '../internal/shared-neo4j';
2727
import {ServerVersion} from '../../src/v1/internal/server-version';
28+
import lolex from 'lolex';
2829

2930
describe('connector', () => {
3031

32+
let clock;
3133
let connection;
3234

3335
afterEach(done => {
36+
if (clock) {
37+
clock.uninstall();
38+
clock = null;
39+
}
40+
3441
const usedConnection = connection;
3542
connection = null;
3643
if (usedConnection) {
@@ -39,6 +46,15 @@ describe('connector', () => {
3946
done();
4047
});
4148

49+
it('should have correct creation timestamp', () => {
50+
clock = lolex.install();
51+
clock.setSystemTime(424242);
52+
53+
connection = connect('bolt://localhost');
54+
55+
expect(connection.creationTimestamp).toEqual(424242);
56+
});
57+
4258
it('should read/write basic messages', done => {
4359
// Given
4460
connection = connect("bolt://localhost");

test/internal/fake-connection.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
export default class FakeConnection {
2828

2929
constructor() {
30+
this._open = true;
31+
this.creationTimestamp = Date.now();
32+
3033
this.resetInvoked = 0;
3134
this.resetAsyncInvoked = 0;
3235
this.syncInvoked = 0;
@@ -68,6 +71,10 @@ export default class FakeConnection {
6871
return this._initializationPromise;
6972
}
7073

74+
isOpen() {
75+
return this._open;
76+
}
77+
7178
isReleasedOnceOnSessionClose() {
7279
return this.isReleasedOnSessionCloseTimes(1);
7380
}
@@ -103,4 +110,14 @@ export default class FakeConnection {
103110
this._initializationPromise = Promise.reject(error);
104111
return this;
105112
}
113+
114+
withCreationTimestamp(value) {
115+
this.creationTimestamp = value;
116+
return this;
117+
}
118+
119+
closed() {
120+
this._open = false;
121+
return this;
122+
}
106123
};

test/internal/routing-util.test.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@ describe('RoutingUtil', () => {
3030

3131
let clock;
3232

33-
beforeAll(() => {
34-
clock = lolex.install();
35-
});
36-
37-
afterAll(() => {
38-
clock.uninstall();
33+
afterEach(() => {
34+
if (clock) {
35+
clock.uninstall();
36+
clock = null;
37+
}
3938
});
4039

4140
it('should return retrieved records when query succeeds', done => {
@@ -141,6 +140,8 @@ describe('RoutingUtil', () => {
141140
});
142141

143142
it('should parse valid ttl', () => {
143+
clock = lolex.install();
144+
144145
testValidTtlParsing(100, 5);
145146
testValidTtlParsing(Date.now(), 3600); // 1 hour
146147
testValidTtlParsing(Date.now(), 86400); // 24 hours
@@ -152,6 +153,7 @@ describe('RoutingUtil', () => {
152153

153154
it('should not overflow parsing huge ttl', () => {
154155
const record = newRecord({ttl: Integer.MAX_VALUE});
156+
clock = lolex.install();
155157
clock.setSystemTime(42);
156158

157159
const expirationTime = parseTtl(record);
@@ -161,6 +163,7 @@ describe('RoutingUtil', () => {
161163

162164
it('should return valid value parsing negative ttl', () => {
163165
const record = newRecord({ttl: int(-42)});
166+
clock = lolex.install();
164167
clock.setSystemTime(42);
165168

166169
const expirationTime = parseTtl(record);

test/internal/timers-util.js

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,3 @@ class SetTimeoutMock {
6666
}
6767

6868
export const setTimeoutMock = new SetTimeoutMock();
69-
70-
export function hijackNextDateNowCall(newValue) {
71-
const originalDate = global.Date;
72-
global.Date = new FakeDate(originalDate, newValue);
73-
}
74-
75-
class FakeDate {
76-
77-
constructor(originalDate, nextNowValue) {
78-
this._originalDate = originalDate;
79-
this._nextNowValue = nextNowValue;
80-
}
81-
82-
now() {
83-
global.Date = this._originalDate;
84-
return this._nextNowValue;
85-
}
86-
}

test/internal/transaction-executor.test.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020
import TransactionExecutor from '../../src/v1/internal/transaction-executor';
2121
import {newError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../../src/v1/error';
22-
import {hijackNextDateNowCall, setTimeoutMock} from './timers-util';
22+
import {setTimeoutMock} from './timers-util';
23+
import lolex from 'lolex';
2324

2425
const TRANSIENT_ERROR_1 = 'Neo.TransientError.Transaction.DeadlockDetected';
2526
const TRANSIENT_ERROR_2 = 'Neo.TransientError.Network.CommunicationError';
@@ -30,13 +31,18 @@ const OOM_ERROR = 'Neo.DatabaseError.General.OutOfMemoryError';
3031

3132
describe('TransactionExecutor', () => {
3233

34+
let clock;
3335
let fakeSetTimeout;
3436

3537
beforeEach(() => {
3638
fakeSetTimeout = setTimeoutMock.install();
3739
});
3840

3941
afterEach(() => {
42+
if (clock) {
43+
clock.uninstall();
44+
clock = null;
45+
}
4046
fakeSetTimeout.uninstall();
4147
});
4248

@@ -81,7 +87,9 @@ describe('TransactionExecutor', () => {
8187
expect(tx).toBeDefined();
8288
workInvocationCounter++;
8389
if (workInvocationCounter === 3) {
84-
hijackNextDateNowCall(Date.now() + 30001); // move next `Date.now()` call forward by 30 seconds
90+
const currentTime = Date.now();
91+
clock = lolex.install();
92+
clock.setSystemTime(currentTime + 30001); // move `Date.now()` call forward by 30 seconds
8593
}
8694
return realWork();
8795
});

0 commit comments

Comments
 (0)