diff --git a/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js b/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js index fb32b1ca6..d15de0cd9 100644 --- a/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js +++ b/packages/bolt-connection/src/connection-provider/connection-provider-pooled.js @@ -20,6 +20,7 @@ import Pool, { PoolConfig } from '../pool' import { error, ConnectionProvider, ServerInfo, newError } from 'neo4j-driver-core' import AuthenticationProvider from './authentication-provider' import { object } from '../lang' +import LivenessCheckProvider from './liveness-check-provider' const { SERVICE_UNAVAILABLE } = error const AUTHENTICATION_ERRORS = [ @@ -40,6 +41,7 @@ export default class PooledConnectionProvider extends ConnectionProvider { this._config = config this._log = log this._authenticationProvider = new AuthenticationProvider({ authTokenManager, userAgent, boltAgent }) + this._livenessCheckProvider = new LivenessCheckProvider({ connectionLivenessCheckTimeout: config.connectionLivenessCheckTimeout }) this._userAgent = userAgent this._boltAgent = boltAgent this._createChannelConnection = @@ -81,6 +83,7 @@ export default class PooledConnectionProvider extends ConnectionProvider { _createConnection ({ auth }, address, release) { return this._createChannelConnection(address).then(connection => { connection.release = () => { + connection.idleTimestamp = Date.now() return release(address, connection) } this._openConnections[connection.id] = connection @@ -100,6 +103,15 @@ export default class PooledConnectionProvider extends ConnectionProvider { return false } + try { + await this._livenessCheckProvider.check(conn) + } catch (error) { + this._log.debug( + `The connection ${conn.id} is not alive because of an error ${error.code} '${error.message}'` + ) + return false + } + try { await this._authenticationProvider.authenticate({ connection: conn, auth, skipReAuth }) return true diff --git a/packages/bolt-connection/src/connection-provider/liveness-check-provider.js b/packages/bolt-connection/src/connection-provider/liveness-check-provider.js new file mode 100644 index 000000000..4d360b635 --- /dev/null +++ b/packages/bolt-connection/src/connection-provider/liveness-check-provider.js @@ -0,0 +1,51 @@ +/** + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [https://neo4j.com] + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export default class LivenessCheckProvider { + constructor ({ connectionLivenessCheckTimeout }) { + this._connectionLivenessCheckTimeout = connectionLivenessCheckTimeout + } + + /** + * Checks connection liveness with configured params. + * + * @param {Connection} connection + * @returns {Promise} If liveness checks succeed, throws otherwise + */ + async check (connection) { + if (this._isCheckDisabled || this._isNewlyCreatedConnection(connection)) { + return true + } + + const idleFor = Date.now() - connection.idleTimestamp + + if (this._connectionLivenessCheckTimeout === 0 || + idleFor > this._connectionLivenessCheckTimeout) { + return await connection.resetAndFlush() + .then(() => true) + } + + return true + } + + get _isCheckDisabled () { + return this._connectionLivenessCheckTimeout == null || this._connectionLivenessCheckTimeout < 0 + } + + _isNewlyCreatedConnection (connection) { + return connection.authToken == null + } +} diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js index fd3ec73d2..f291b7ec3 100644 --- a/packages/bolt-connection/src/connection/connection-channel.js +++ b/packages/bolt-connection/src/connection/connection-channel.js @@ -130,7 +130,7 @@ export default class ChannelConnection extends Connection { this._id = idGenerator++ this._address = address this._server = { address: address.asHostPort() } - this.creationTimestamp = Date.now() + this._creationTimestamp = Date.now() this._disableLosslessIntegers = disableLosslessIntegers this._ch = channel this._chunker = chunker @@ -220,6 +220,18 @@ export default class ChannelConnection extends Connection { this._dbConnectionId = value } + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + + get creationTimestamp () { + return this._creationTimestamp + } + /** * Send initialization message. * @param {string} userAgent the user agent for this driver. diff --git a/packages/bolt-connection/src/connection/connection-delegate.js b/packages/bolt-connection/src/connection/connection-delegate.js index 14c005cb0..ea24c1210 100644 --- a/packages/bolt-connection/src/connection/connection-delegate.js +++ b/packages/bolt-connection/src/connection/connection-delegate.js @@ -93,6 +93,18 @@ export default class DelegateConnection extends Connection { this._delegate.version = value } + get creationTimestamp () { + return this._delegate.creationTimestamp + } + + set idleTimestamp (value) { + this._delegate.idleTimestamp = value + } + + get idleTimestamp () { + return this._delegate.idleTimestamp + } + isOpen () { return this._delegate.isOpen() } diff --git a/packages/bolt-connection/src/connection/connection.js b/packages/bolt-connection/src/connection/connection.js index 9e8f0bcff..a77619a3b 100644 --- a/packages/bolt-connection/src/connection/connection.js +++ b/packages/bolt-connection/src/connection/connection.js @@ -51,6 +51,18 @@ export default class Connection extends CoreConnection { throw new Error('not implemented') } + get creationTimestamp () { + throw new Error('not implemented') + } + + set idleTimestamp (value) { + throw new Error('not implemented') + } + + get idleTimestamp () { + throw new Error('not implemented') + } + /** * @returns {BoltProtocol} the underlying bolt protocol assigned to this connection */ diff --git a/packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js b/packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js index 24b9ef935..ff2f10ee0 100644 --- a/packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js +++ b/packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js @@ -21,6 +21,7 @@ import { Connection, DelegateConnection } from '../../src/connection' import { authTokenManagers, internal, newError, ServerInfo, staticAuthTokenManager } from 'neo4j-driver-core' import AuthenticationProvider from '../../src/connection-provider/authentication-provider' import { functional } from '../../src/lang' +import LivenessCheckProvider from '../../src/connection-provider/liveness-check-provider' const { serverAddress: { ServerAddress }, @@ -281,16 +282,23 @@ describe('constructor', () => { }) it('should register the release function into the connection', async () => { - const { create } = setup() - const releaseResult = { property: 'some property' } - const release = jest.fn(() => releaseResult) + jest.useFakeTimers() + try { + const { create } = setup() + const releaseResult = { property: 'some property' } + const release = jest.fn(() => releaseResult) - const connection = await create({}, server0, release) + const connection = await create({}, server0, release) + connection.idleTimestamp = -1234 - const released = connection.release() + const released = connection.release() - expect(released).toBe(releaseResult) - expect(release).toHaveBeenCalledWith(server0, connection) + expect(released).toBe(releaseResult) + expect(release).toHaveBeenCalledWith(server0, connection) + expect(connection.idleTimestamp).toBeCloseTo(Date.now()) + } finally { + jest.useRealTimers() + } }) it.each([ @@ -361,26 +369,27 @@ describe('constructor', () => { const connection = new FakeConnection(server0) connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({ auth }, connection)).resolves.toBe(true) expect(authenticationProviderHook.authenticate).toHaveBeenCalledWith({ connection, auth }) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) }) it.each([ null, undefined, { scheme: 'bearer', credentials: 'token01' } - ])('should return true when connection is open and within the lifetime and authentication fails (auth=%o)', async (auth) => { + ])('should return false when connection is open and within the lifetime and authentication fails (auth=%o)', async (auth) => { const connection = new FakeConnection(server0) const error = newError('failed') const authenticationProvider = jest.fn(() => Promise.reject(error)) connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook, log } = setup({ authenticationProvider }) + const { validateOnAcquire, authenticationProviderHook, log, livenessCheckProviderHook } = setup({ authenticationProvider }) await expect(validateOnAcquire({ auth }, connection)).resolves.toBe(false) @@ -391,6 +400,7 @@ describe('constructor', () => { expect(log.debug).toHaveBeenCalledWith( `The connection ${connection.id} is not valid because of an error ${error.code} '${error.message}'` ) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) }) it.each([ @@ -401,13 +411,32 @@ describe('constructor', () => { const auth = {} connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({ auth, skipReAuth }, connection)).resolves.toBe(true) expect(authenticationProviderHook.authenticate).toHaveBeenCalledWith({ connection, auth, skipReAuth }) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) + }) + + it('should return false when liveness checks fails', async () => { + const connection = new FakeConnection(server0) + connection.creationTimestamp = Date.now() + const error = newError('#themessage', '#thecode') + + const { validateOnAcquire, authenticationProviderHook, log, livenessCheckProviderHook } = setup({ + livenessCheckProvider: () => Promise.reject(error) + }) + + await expect(validateOnAcquire({}, connection)).resolves.toBe(false) + + expect(livenessCheckProviderHook.check).toBeCalledWith(connection) + expect(log.debug).toBeCalledWith( + `The connection ${connection.id} is not alive because of an error ${error.code} '${error.message}'` + ) + expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() }) it('should return false when connection is closed and within the lifetime', async () => { @@ -415,20 +444,22 @@ describe('constructor', () => { connection.creationTimestamp = Date.now() await connection.close() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) it('should return false when connection is open and out of the lifetime', async () => { const connection = new FakeConnection(server0) connection.creationTimestamp = Date.now() - 4000 - const { validateOnAcquire, authenticationProviderHook } = setup({ maxConnectionLifetime: 3000 }) + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup({ maxConnectionLifetime: 3000 }) await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) it('should return false when connection is closed and out of the lifetime', async () => { @@ -436,10 +467,11 @@ describe('constructor', () => { await connection.close() connection.creationTimestamp = Date.now() - 4000 - const { validateOnAcquire, authenticationProviderHook } = setup({ maxConnectionLifetime: 3000 }) + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup({ maxConnectionLifetime: 3000 }) await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) }) @@ -492,14 +524,17 @@ describe('constructor', () => { }) }) - function setup ({ createConnection, authenticationProvider, maxConnectionLifetime } = {}) { + function setup ({ createConnection, authenticationProvider, maxConnectionLifetime, livenessCheckProvider } = {}) { const newPool = jest.fn((...args) => new Pool(...args)) const log = new Logger('debug', () => undefined) jest.spyOn(log, 'debug') const createChannelConnectionHook = createConnection || jest.fn(async (address) => new FakeConnection(address)) const authenticationProviderHook = new AuthenticationProvider({ }) + const livenessCheckProviderHook = new LivenessCheckProvider({}) jest.spyOn(authenticationProviderHook, 'authenticate') .mockImplementation(authenticationProvider || jest.fn(({ connection }) => Promise.resolve(connection))) + jest.spyOn(livenessCheckProviderHook, 'check') + .mockImplementation(livenessCheckProvider || jest.fn(() => Promise.resolve(true))) const provider = new DirectConnectionProvider({ newPool, config: { @@ -510,11 +545,13 @@ describe('constructor', () => { }) provider._createChannelConnection = createChannelConnectionHook provider._authenticationProvider = authenticationProviderHook + provider._livenessCheckProvider = livenessCheckProviderHook return { provider, ...newPool.mock.calls[0][0], createChannelConnectionHook, authenticationProviderHook, + livenessCheckProviderHook, log } } @@ -812,6 +849,22 @@ class FakeConnection extends Connection { return this._supportsReAuth } + set creationTimestamp (value) { + this._creationTimestamp = value + } + + get creationTimestamp () { + return this._creationTimestamp + } + + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + async close () { this._closed = true } diff --git a/packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js b/packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js index e93835617..c1eeeb794 100644 --- a/packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js +++ b/packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js @@ -33,6 +33,7 @@ import RoutingConnectionProvider from '../../src/connection-provider/connection- import { DelegateConnection, Connection } from '../../src/connection' import AuthenticationProvider from '../../src/connection-provider/authentication-provider' import { functional } from '../../src/lang' +import LivenessCheckProvider from '../../src/connection-provider/liveness-check-provider' const { serverAddress: { ServerAddress }, @@ -3185,16 +3186,23 @@ describe.each([ }) it('should register the release function into the connection', async () => { - const { create } = setup() - const releaseResult = { property: 'some property' } - const release = jest.fn(() => releaseResult) + jest.useFakeTimers() + try { + const { create } = setup() + const releaseResult = { property: 'some property' } + const release = jest.fn(() => releaseResult) - const connection = await create({}, server0, release) + const connection = await create({}, server0, release) + connection.idleTimestamp = -1234 - const released = connection.release() + const released = connection.release() - expect(released).toBe(releaseResult) - expect(release).toHaveBeenCalledWith(server0, connection) + expect(released).toBe(releaseResult) + expect(release).toHaveBeenCalledWith(server0, connection) + expect(connection.idleTimestamp).toBeCloseTo(Date.now()) + } finally { + jest.useRealTimers() + } }) it.each([ @@ -3265,13 +3273,14 @@ describe.each([ const connection = new FakeConnection(server0) connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({ auth }, connection)).resolves.toBe(true) expect(authenticationProviderHook.authenticate).toHaveBeenCalledWith({ connection, auth }) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) }) it.each([ @@ -3284,7 +3293,7 @@ describe.each([ const authenticationProvider = jest.fn(() => Promise.reject(error)) connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook, log } = setup({ authenticationProvider }) + const { validateOnAcquire, authenticationProviderHook, log, livenessCheckProviderHook } = setup({ authenticationProvider }) await expect(validateOnAcquire({ auth }, connection)).resolves.toBe(false) @@ -3295,6 +3304,7 @@ describe.each([ expect(log.debug).toHaveBeenCalledWith( `The connection ${connection.id} is not valid because of an error ${error.code} '${error.message}'` ) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) }) it.each([ @@ -3305,13 +3315,32 @@ describe.each([ const auth = {} connection.creationTimestamp = Date.now() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({ auth, skipReAuth }, connection)).resolves.toBe(true) expect(authenticationProviderHook.authenticate).toHaveBeenCalledWith({ connection, auth, skipReAuth }) + expect(livenessCheckProviderHook.check).toHaveBeenCalledWith(connection) + }) + + it('should return false when liveness checks fails', async () => { + const connection = new FakeConnection(server0) + connection.creationTimestamp = Date.now() + const error = newError('#themessage', '#thecode') + + const { validateOnAcquire, authenticationProviderHook, log, livenessCheckProviderHook } = setup({ + livenessCheckProvider: () => Promise.reject(error) + }) + + await expect(validateOnAcquire({}, connection)).resolves.toBe(false) + + expect(livenessCheckProviderHook.check).toBeCalledWith(connection) + expect(log.debug).toBeCalledWith( + `The connection ${connection.id} is not alive because of an error ${error.code} '${error.message}'` + ) + expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() }) it('should return false when connection is closed and within the lifetime', async () => { @@ -3319,20 +3348,22 @@ describe.each([ connection.creationTimestamp = Date.now() await connection.close() - const { validateOnAcquire, authenticationProviderHook } = setup() + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup() await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) it('should return false when connection is open and out of the lifetime', async () => { const connection = new FakeConnection(server0) connection.creationTimestamp = Date.now() - 4000 - const { validateOnAcquire, authenticationProviderHook } = setup({ maxConnectionLifetime: 3000 }) + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup({ maxConnectionLifetime: 3000 }) await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) it('should return false when connection is closed and out of the lifetime', async () => { @@ -3340,10 +3371,11 @@ describe.each([ await connection.close() connection.creationTimestamp = Date.now() - 4000 - const { validateOnAcquire, authenticationProviderHook } = setup({ maxConnectionLifetime: 3000 }) + const { validateOnAcquire, authenticationProviderHook, livenessCheckProviderHook } = setup({ maxConnectionLifetime: 3000 }) await expect(validateOnAcquire({}, connection)).resolves.toBe(false) expect(authenticationProviderHook.authenticate).not.toHaveBeenCalled() + expect(livenessCheckProviderHook.check).not.toHaveBeenCalled() }) }) @@ -3396,14 +3428,17 @@ describe.each([ }) }) - function setup ({ createConnection, authenticationProvider, maxConnectionLifetime } = {}) { + function setup ({ createConnection, authenticationProvider, maxConnectionLifetime, livenessCheckProvider } = {}) { const newPool = jest.fn((...args) => new Pool(...args)) const log = new Logger('debug', () => undefined) jest.spyOn(log, 'debug') const createChannelConnectionHook = createConnection || jest.fn(async (address) => new FakeConnection(address)) const authenticationProviderHook = new AuthenticationProvider({}) + const livenessCheckProviderHook = new LivenessCheckProvider({}) jest.spyOn(authenticationProviderHook, 'authenticate') .mockImplementation(authenticationProvider || jest.fn(({ connection }) => Promise.resolve(connection))) + jest.spyOn(livenessCheckProviderHook, 'check') + .mockImplementation(livenessCheckProvider || jest.fn(() => Promise.resolve(true))) const provider = new RoutingConnectionProvider({ newPool, config: { @@ -3414,11 +3449,13 @@ describe.each([ }) provider._createChannelConnection = createChannelConnectionHook provider._authenticationProvider = authenticationProviderHook + provider._livenessCheckProvider = livenessCheckProviderHook return { provider, ...newPool.mock.calls[0][0], createChannelConnectionHook, authenticationProviderHook, + livenessCheckProviderHook, log } } @@ -3942,6 +3979,22 @@ class FakeConnection extends Connection { this._closed = true } + set creationTimestamp (value) { + this._creationTimestamp = value + } + + get creationTimestamp () { + return this._creationTimestamp + } + + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + isOpen () { return !this._closed } diff --git a/packages/bolt-connection/test/connection-provider/liveness-check-provider.test.js b/packages/bolt-connection/test/connection-provider/liveness-check-provider.test.js new file mode 100644 index 000000000..2a3eacfa9 --- /dev/null +++ b/packages/bolt-connection/test/connection-provider/liveness-check-provider.test.js @@ -0,0 +1,152 @@ +/** + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [https://neo4j.com] + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { newError } from 'neo4j-driver-core' +import LivenessCheckProvider from '../../src/connection-provider/liveness-check-provider' + +describe('LivenessCheckProvider', () => { + describe('.check', () => { + describe.each(noNetworkNeededFixture())('when connectionLivenessCheckTimeout=%s, connection.authToken=%s and idleFor=%s', (connectionLivenessCheckTimeout, authToken, idleFor) => { + it('should return resolves with true', async () => { + const { provider, connection } = scenario() + + await expect(provider.check(connection)).resolves.toBe(true) + }) + + it('should not reset connection', async () => { + const { provider, connection } = scenario() + + await provider.check(connection) + + expect(connection.resetAndFlush).not.toHaveBeenCalled() + }) + + function scenario () { + const provider = new LivenessCheckProvider({ + connectionLivenessCheckTimeout + }) + + const connection = mockConnection({ authToken }) + + return { provider, connection } + } + }) + + describe.each(networkNeededFixture())('when connectionLivenessCheckTimeout=%s, connection.authToken=%s and idleFor=%s', (connectionLivenessCheckTimeout, authToken, idleFor) => { + describe('and resetAndFlush succeed', () => { + it('should return resolves with true', async () => { + const { provider, connection } = scenario() + + await expect(provider.check(connection)).resolves.toBe(true) + }) + + it('should reset connection once', async () => { + const { provider, connection } = scenario() + + await provider.check(connection).catch(() => {}) + + expect(connection.resetAndFlush).toHaveBeenCalledTimes(1) + }) + + function scenario () { + const provider = new LivenessCheckProvider({ + connectionLivenessCheckTimeout + }) + + const connection = mockConnection({ + authToken, + idleFor + }) + + return { provider, connection } + } + }) + + describe('and resetAndFlush fail', () => { + const error = newError('Something wrong is not right') + + it('should reject with expected error', async () => { + const { provider, connection } = scenario() + + await expect(provider.check(connection)).rejects.toBe(error) + }) + + it('should reset connection once', async () => { + const { provider, connection } = scenario() + + await provider.check(connection).catch(() => {}) + + expect(connection.resetAndFlush).toHaveBeenCalledTimes(1) + }) + + function scenario () { + const provider = new LivenessCheckProvider({ + connectionLivenessCheckTimeout + }) + + const connection = mockConnection({ + authToken, + idleFor, + resetAndFlushPromise: Promise.reject(error) + }) + + return { provider, connection } + } + }) + }) + }) + + function noNetworkNeededFixture () { + // [connectionLivenessCheckTimeout, authToken, idleFor] + return [ + [undefined, null, 1245], + [null, null, 30000], + [-1, undefined, 30000], + [undefined, undefined, 30000], + [null, undefined, 30000], + [-1, { scheme: 'none' }, 30000], + [undefined, { scheme: 'none' }, 30000], + [null, { scheme: 'none' }, 30000], + [0, undefined, 30000], + [0, null, 30000], + [123, undefined, 30000], + [3123, null, 30000], + [30000, { scheme: 'none' }, 30000], + [29999, { scheme: 'none' }, 30000], + [1, { scheme: 'none' }, 30000] + ] + } + + function networkNeededFixture () { + // [connectionLivenessCheckTimeout, authToken, idleFor] + return [ + [0, { scheme: 'none' }, 0], + [0, { scheme: 'none' }, 1234], + [0, { scheme: 'none' }, 3234], + [1000, { scheme: 'none' }, 3234], + [3233, { scheme: 'none' }, 3234] + ] + } + + function mockConnection ({ resetAndFlushPromise, authToken, idleFor } = {}) { + return { + resetAndFlush: jest.fn(() => resetAndFlushPromise || Promise.resolve()), + authToken, + idleTimestamp: idleFor ? Date.now() - idleFor : undefined + } + } +}) diff --git a/packages/bolt-connection/test/fake-connection.js b/packages/bolt-connection/test/fake-connection.js index 18f99e595..80f446d93 100644 --- a/packages/bolt-connection/test/fake-connection.js +++ b/packages/bolt-connection/test/fake-connection.js @@ -32,7 +32,7 @@ export default class FakeConnection extends Connection { this._id = 0 this._databaseId = null this._requestRoutingInformationMock = null - this.creationTimestamp = Date.now() + this._creationTimestamp = Date.now() this.resetInvoked = 0 this.releaseInvoked = 0 @@ -70,6 +70,22 @@ export default class FakeConnection extends Connection { this._server.version = value } + set creationTimestamp (value) { + this._creationTimestamp = value + } + + get creationTimestamp () { + return this._creationTimestamp + } + + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + protocol () { // return fake protocol object that simply records seen queries and parameters return { diff --git a/packages/core/src/driver.ts b/packages/core/src/driver.ts index 217188d34..96d1fdeec 100644 --- a/packages/core/src/driver.ts +++ b/packages/core/src/driver.ts @@ -107,6 +107,7 @@ interface DriverConfig { fetchSize?: number logging?: LoggingConfig notificationFilter?: NotificationFilter + connectionLivenessCheckTimeout?: number } /** @@ -907,6 +908,9 @@ function sanitizeConfig (config: any): void { DEFAULT_FETCH_SIZE ) config.connectionTimeout = extractConnectionTimeout(config) + + config.connectionLivenessCheckTimeout = + validateConnectionLivenessCheckTimeoutSizeValue(config.connectionLivenessCheckTimeout) } /** @@ -962,6 +966,24 @@ function extractConnectionTimeout (config: any): number | null { } } +/** + * @private + */ +function validateConnectionLivenessCheckTimeoutSizeValue ( + rawValue: any +): number | undefined { + if (rawValue == null) { + return undefined + } + const connectionLivenessCheckTimeout = parseInt(rawValue, 10) + if (connectionLivenessCheckTimeout < 0 || Number.isNaN(connectionLivenessCheckTimeout)) { + throw new Error( + `The connectionLivenessCheckTimeout can only be a positive value or 0 for always. However connectionLivenessCheckTimeout = ${connectionLivenessCheckTimeout}` + ) + } + return connectionLivenessCheckTimeout +} + /** * @private * @returns {ConfiguredCustomResolver} new custom resolver that wraps the passed-in resolver function. diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 1e422ff4f..05e07d134 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -71,6 +71,7 @@ export class Config { maxTransactionRetryTime?: number maxConnectionLifetime?: number connectionAcquisitionTimeout?: number + connectionLivenessCheckTimeout?: number connectionTimeout?: number disableLosslessIntegers?: boolean notificationFilter?: NotificationFilter @@ -184,6 +185,33 @@ export class Config { */ this.maxTransactionRetryTime = 30000 // 30 seconds + /** + * Specify the maximum time in milliseconds the connection can be idle without needing + * to perform a liveness check on acquire from the pool. + * + * Pooled connections that have been idle in the pool for longer than this + * timeout will be tested before they are used again, to ensure they are still live. + * If this option is set too low, an additional network call will be incurred + * when acquiring a connection, which causes a performance hit. + * + * If this is set high, you may receive sessions that are backed by no longer + * live connections, which will lead to exceptions in your application. + * Assuming the database is running, these exceptions will go away if you retry + * acquiring sessions. + * + * Hence, this parameter tunes a balance between the likelihood of your application + * seeing connection problems, and performance. + * + * You normally should not need to tune this parameter. No connection liveliness + * check is done by default. Value 0 means connections will always be tested for + * validity and negative values mean connections will never be tested. + * + * **Default**: ```undefined``` (Disabled) + * + * @type {number|undefined} + */ + this.connectionLivenessCheckTimeout = undefined // Disabled + /** * Specify socket connection timeout in milliseconds. * diff --git a/packages/core/test/driver.test.ts b/packages/core/test/driver.test.ts index fd5c9a16d..48b872703 100644 --- a/packages/core/test/driver.test.ts +++ b/packages/core/test/driver.test.ts @@ -15,7 +15,7 @@ * limitations under the License. */ /* eslint-disable @typescript-eslint/promise-function-async */ -import { bookmarkManager, ConnectionProvider, EagerResult, newError, NotificationFilter, Result, ResultSummary, ServerInfo, Session, TransactionConfig } from '../src' +import { bookmarkManager, ConnectionProvider, EagerResult, int, newError, NotificationFilter, Result, ResultSummary, ServerInfo, Session, TransactionConfig } from '../src' import Driver, { QueryConfig, READ, routing } from '../src/driver' import { Bookmarks } from '../src/internal/bookmarks' import { Logger } from '../src/internal/logger' @@ -603,6 +603,64 @@ describe('Driver', () => { await driver.close() }) }) + + describe('config', () => { + describe('connectionLivenessCheckTimeout', () => { + it.each([ + [undefined, undefined], + [null, undefined], + [0, 0], + [BigInt(0), 0], + [int(0), 0], + ['0', 0], + [3000, 3000], + [BigInt(3000), 3000], + [int(3000), 3000], + ['3000', 3000] + ])('should sanitize value % to %s', async (configured, expected) => { + const createConnectionProviderMock = jest.fn(mockCreateConnectonProvider(connectionProvider)) + const driver = new Driver( + META_INFO, + // @ts-expect-error + { connectionLivenessCheckTimeout: configured }, + createConnectionProviderMock, + createSession + ) + + driver._getOrCreateConnectionProvider() + + expect(createConnectionProviderMock).toHaveBeenCalledWith( + expect.any(Number), + expect.objectContaining({ connectionLivenessCheckTimeout: expected }), + expect.any(Object), + expect.any(Object) + ) + + await driver.close() + }) + + it.each([ + [-1, -1], + [int(-1), -1], + [BigInt(-1), -1], + ['-1', -1], + [{}, NaN], + ['a', NaN] + ])('should fail in invalid values', async (configured, expected) => { + const createConnectionProviderMock = jest.fn(mockCreateConnectonProvider(connectionProvider)) + + expect(() => new Driver( + META_INFO, + // @ts-expect-error + { connectionLivenessCheckTimeout: configured }, + createConnectionProviderMock, + createSession + )).toThrow(new Error(`The connectionLivenessCheckTimeout can only be a positive value or 0 for always. However connectionLivenessCheckTimeout = ${expected}`)) + + expect(createConnectionProviderMock).not.toHaveBeenCalled() + }) + }) + }) }) function mockCreateConnectonProvider (connectionProvider: ConnectionProvider) { diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js index bb1332cbd..0476cc687 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-pooled.js @@ -20,6 +20,7 @@ import Pool, { PoolConfig } from '../pool/index.js' import { error, ConnectionProvider, ServerInfo, newError } from '../../core/index.ts' import AuthenticationProvider from './authentication-provider.js' import { object } from '../lang/index.js' +import LivenessCheckProvider from './liveness-check-provider.js' const { SERVICE_UNAVAILABLE } = error const AUTHENTICATION_ERRORS = [ @@ -40,6 +41,7 @@ export default class PooledConnectionProvider extends ConnectionProvider { this._config = config this._log = log this._authenticationProvider = new AuthenticationProvider({ authTokenManager, userAgent, boltAgent }) + this._livenessCheckProvider = new LivenessCheckProvider({ connectionLivenessCheckTimeout: config.connectionLivenessCheckTimeout }) this._userAgent = userAgent this._boltAgent = boltAgent this._createChannelConnection = @@ -81,6 +83,7 @@ export default class PooledConnectionProvider extends ConnectionProvider { _createConnection ({ auth }, address, release) { return this._createChannelConnection(address).then(connection => { connection.release = () => { + connection.idleTimestamp = Date.now() return release(address, connection) } this._openConnections[connection.id] = connection @@ -100,6 +103,15 @@ export default class PooledConnectionProvider extends ConnectionProvider { return false } + try { + await this._livenessCheckProvider.check(conn) + } catch (error) { + this._log.debug( + `The connection ${conn.id} is not alive because of an error ${error.code} '${error.message}'` + ) + return false + } + try { await this._authenticationProvider.authenticate({ connection: conn, auth, skipReAuth }) return true diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/liveness-check-provider.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/liveness-check-provider.js new file mode 100644 index 000000000..4d360b635 --- /dev/null +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/liveness-check-provider.js @@ -0,0 +1,51 @@ +/** + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [https://neo4j.com] + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export default class LivenessCheckProvider { + constructor ({ connectionLivenessCheckTimeout }) { + this._connectionLivenessCheckTimeout = connectionLivenessCheckTimeout + } + + /** + * Checks connection liveness with configured params. + * + * @param {Connection} connection + * @returns {Promise} If liveness checks succeed, throws otherwise + */ + async check (connection) { + if (this._isCheckDisabled || this._isNewlyCreatedConnection(connection)) { + return true + } + + const idleFor = Date.now() - connection.idleTimestamp + + if (this._connectionLivenessCheckTimeout === 0 || + idleFor > this._connectionLivenessCheckTimeout) { + return await connection.resetAndFlush() + .then(() => true) + } + + return true + } + + get _isCheckDisabled () { + return this._connectionLivenessCheckTimeout == null || this._connectionLivenessCheckTimeout < 0 + } + + _isNewlyCreatedConnection (connection) { + return connection.authToken == null + } +} diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js index 10de55429..6a4dbe193 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js @@ -130,7 +130,7 @@ export default class ChannelConnection extends Connection { this._id = idGenerator++ this._address = address this._server = { address: address.asHostPort() } - this.creationTimestamp = Date.now() + this._creationTimestamp = Date.now() this._disableLosslessIntegers = disableLosslessIntegers this._ch = channel this._chunker = chunker @@ -220,6 +220,18 @@ export default class ChannelConnection extends Connection { this._dbConnectionId = value } + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + + get creationTimestamp () { + return this._creationTimestamp + } + /** * Send initialization message. * @param {string} userAgent the user agent for this driver. diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-delegate.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-delegate.js index 425ee1ae2..6388f7a6a 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-delegate.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-delegate.js @@ -93,6 +93,18 @@ export default class DelegateConnection extends Connection { this._delegate.version = value } + get creationTimestamp () { + return this._delegate.creationTimestamp + } + + set idleTimestamp (value) { + this._delegate.idleTimestamp = value + } + + get idleTimestamp () { + return this._delegate.idleTimestamp + } + isOpen () { return this._delegate.isOpen() } diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection.js index 5261be0c4..d23c18188 100644 --- a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection.js +++ b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection.js @@ -51,6 +51,18 @@ export default class Connection extends CoreConnection { throw new Error('not implemented') } + get creationTimestamp () { + throw new Error('not implemented') + } + + set idleTimestamp (value) { + throw new Error('not implemented') + } + + get idleTimestamp () { + throw new Error('not implemented') + } + /** * @returns {BoltProtocol} the underlying bolt protocol assigned to this connection */ diff --git a/packages/neo4j-driver-deno/lib/core/driver.ts b/packages/neo4j-driver-deno/lib/core/driver.ts index df2fe34dc..a6e61c49b 100644 --- a/packages/neo4j-driver-deno/lib/core/driver.ts +++ b/packages/neo4j-driver-deno/lib/core/driver.ts @@ -107,6 +107,7 @@ interface DriverConfig { fetchSize?: number logging?: LoggingConfig notificationFilter?: NotificationFilter + connectionLivenessCheckTimeout?: number } /** @@ -907,6 +908,9 @@ function sanitizeConfig (config: any): void { DEFAULT_FETCH_SIZE ) config.connectionTimeout = extractConnectionTimeout(config) + + config.connectionLivenessCheckTimeout = + validateConnectionLivenessCheckTimeoutSizeValue(config.connectionLivenessCheckTimeout) } /** @@ -962,6 +966,24 @@ function extractConnectionTimeout (config: any): number | null { } } +/** + * @private + */ +function validateConnectionLivenessCheckTimeoutSizeValue ( + rawValue: any +): number | undefined { + if (rawValue == null) { + return undefined + } + const connectionLivenessCheckTimeout = parseInt(rawValue, 10) + if (connectionLivenessCheckTimeout < 0 || Number.isNaN(connectionLivenessCheckTimeout)) { + throw new Error( + `The connectionLivenessCheckTimeout can only be a positive value or 0 for always. However connectionLivenessCheckTimeout = ${connectionLivenessCheckTimeout}` + ) + } + return connectionLivenessCheckTimeout +} + /** * @private * @returns {ConfiguredCustomResolver} new custom resolver that wraps the passed-in resolver function. diff --git a/packages/neo4j-driver-deno/lib/core/types.ts b/packages/neo4j-driver-deno/lib/core/types.ts index 45a4ab67d..f8cc7a735 100644 --- a/packages/neo4j-driver-deno/lib/core/types.ts +++ b/packages/neo4j-driver-deno/lib/core/types.ts @@ -71,6 +71,7 @@ export class Config { maxTransactionRetryTime?: number maxConnectionLifetime?: number connectionAcquisitionTimeout?: number + connectionLivenessCheckTimeout?: number connectionTimeout?: number disableLosslessIntegers?: boolean notificationFilter?: NotificationFilter @@ -184,6 +185,33 @@ export class Config { */ this.maxTransactionRetryTime = 30000 // 30 seconds + /** + * Specify the maximum time in milliseconds the connection can be idle without needing + * to perform a liveness check on acquire from the pool. + * + * Pooled connections that have been idle in the pool for longer than this + * timeout will be tested before they are used again, to ensure they are still live. + * If this option is set too low, an additional network call will be incurred + * when acquiring a connection, which causes a performance hit. + * + * If this is set high, you may receive sessions that are backed by no longer + * live connections, which will lead to exceptions in your application. + * Assuming the database is running, these exceptions will go away if you retry + * acquiring sessions. + * + * Hence, this parameter tunes a balance between the likelihood of your application + * seeing connection problems, and performance. + * + * You normally should not need to tune this parameter. No connection liveliness + * check is done by default. Value 0 means connections will always be tested for + * validity and negative values mean connections will never be tested. + * + * **Default**: ```undefined``` (Disabled) + * + * @type {number|undefined} + */ + this.connectionLivenessCheckTimeout = undefined // Disabled + /** * Specify socket connection timeout in milliseconds. * diff --git a/packages/neo4j-driver/test/internal/fake-connection.js b/packages/neo4j-driver/test/internal/fake-connection.js index 11b7c8e47..82089b968 100644 --- a/packages/neo4j-driver/test/internal/fake-connection.js +++ b/packages/neo4j-driver/test/internal/fake-connection.js @@ -38,7 +38,7 @@ export default class FakeConnection extends Connection { this._id = 0 this._databaseId = null this._requestRoutingInformationMock = null - this.creationTimestamp = Date.now() + this._creationTimestamp = Date.now() this.resetInvoked = 0 this.releaseInvoked = 0 @@ -85,6 +85,22 @@ export default class FakeConnection extends Connection { this._authToken = authToken } + set creationTimestamp (value) { + this._creationTimestamp = value + } + + get creationTimestamp () { + return this._creationTimestamp + } + + set idleTimestamp (value) { + this._idleTimestamp = value + } + + get idleTimestamp () { + return this._idleTimestamp + } + protocol () { // return fake protocol object that simply records seen queries and parameters return { diff --git a/packages/testkit-backend/src/feature/common.js b/packages/testkit-backend/src/feature/common.js index 67887a714..69d881f76 100644 --- a/packages/testkit-backend/src/feature/common.js +++ b/packages/testkit-backend/src/feature/common.js @@ -32,6 +32,7 @@ const features = [ 'Feature:API:Driver.VerifyAuthentication', 'Feature:API:Driver.VerifyConnectivity', 'Feature:API:Session:NotificationsConfig', + 'Feature:API:Liveness.Check', 'Optimization:AuthPipelining', 'Optimization:EagerTransactionBegin', 'Optimization:ExecuteQueryPipelining', diff --git a/packages/testkit-backend/src/request-handlers.js b/packages/testkit-backend/src/request-handlers.js index 19bb4ed23..b1f17dd55 100644 --- a/packages/testkit-backend/src/request-handlers.js +++ b/packages/testkit-backend/src/request-handlers.js @@ -82,6 +82,10 @@ export function NewDriver ({ neo4j }, context, data, wire) { config.telemetryDisabled = data.telemetryDisabled } + if ('livenessCheckTimeoutMs' in data) { + config.connectionLivenessCheckTimeout = data.livenessCheckTimeoutMs + } + let driver try { driver = neo4j.driver(uri, parsedAuthToken, config) diff --git a/packages/testkit-backend/src/skipped-tests/common.js b/packages/testkit-backend/src/skipped-tests/common.js index ee7e6e3bd..4d5eb9669 100644 --- a/packages/testkit-backend/src/skipped-tests/common.js +++ b/packages/testkit-backend/src/skipped-tests/common.js @@ -187,6 +187,12 @@ const skippedTests = [ endsWith('test_error_on_rollback_using_tx_run') ) ) + ), + skip( + 'Behaviour pending unification', + ifStartsWith('stub.routing.test_routing_') + .and( + ifEndsWith('.test_should_drop_connections_failing_liveness_check')) ) ]