diff --git a/src/v1/driver.js b/src/v1/driver.js index 71f76052f..ec6394a26 100644 --- a/src/v1/driver.js +++ b/src/v1/driver.js @@ -25,6 +25,7 @@ import {newError, SERVICE_UNAVAILABLE} from './error'; import {DirectConnectionProvider} from './internal/connection-providers'; import Bookmark from './internal/bookmark'; import ConnectivityVerifier from './internal/connectivity-verifier'; +import PoolConfig from './internal/pool-config'; const READ = 'READ', WRITE = 'WRITE'; /** @@ -60,11 +61,7 @@ class Driver { this._createConnection.bind(this), this._destroyConnection.bind(this), this._validateConnection.bind(this), - { - maxIdleSize: config.connectionPoolSize, - maxSize: config.maxConnectionPoolSize, - acquisitionTimeout: config.connectionAcquisitionTimeout - } + PoolConfig.fromDriverConfig(config) ); /** @@ -243,18 +240,17 @@ class _ConnectionStreamObserver extends StreamObserver { function sanitizeConfig(config) { config.maxConnectionLifetime = sanitizeIntValue(config.maxConnectionLifetime); config.maxConnectionPoolSize = sanitizeIntValue(config.maxConnectionPoolSize); - config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout, 60000); + config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout); } -function sanitizeIntValue(value, defaultValue=null) { +function sanitizeIntValue(value) { if (value) { const sanitizedValue = parseInt(value, 10); if (sanitizedValue && sanitizedValue > 0) { return sanitizedValue; } } - - return defaultValue; + return null; } export {Driver, READ, WRITE} diff --git a/src/v1/index.js b/src/v1/index.js index 922f3fdf5..4265fdb76 100644 --- a/src/v1/index.js +++ b/src/v1/index.js @@ -90,7 +90,7 @@ const USER_AGENT = "neo4j-javascript/" + VERSION; * // as trusted. In the web bundle, this list of trusted certificates is maintained * // by the web browser. In NodeJS, you configure the list with the next config option. * // - * // TRUST_SYSTEM_CA_SIGNED_CERTIFICATES meand that you trust whatever certificates + * // TRUST_SYSTEM_CA_SIGNED_CERTIFICATES means that you trust whatever certificates * // are in the default certificate chain of th * trust: "TRUST_ALL_CERTIFICATES" | "TRUST_ON_FIRST_USE" | "TRUST_SIGNED_CERTIFICATES" | * "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES" | "TRUST_SYSTEM_CA_SIGNED_CERTIFICATES", @@ -110,6 +110,7 @@ const USER_AGENT = "neo4j-javascript/" + VERSION; * * // The max number of connections that are allowed idle in the pool at any time. * // Connection will be destroyed if this threshold is exceeded. + * // Deprecated: please use maxConnectionPoolSize instead. * connectionPoolSize: 50, * * // The maximum total number of connections allowed to be managed by the connection pool, per host. diff --git a/src/v1/internal/ch-node.js b/src/v1/internal/ch-node.js index 7d4fb3458..cb87391d2 100644 --- a/src/v1/internal/ch-node.js +++ b/src/v1/internal/ch-node.js @@ -108,7 +108,7 @@ const TrustStrategy = { * @deprecated Since version 1.0. Will be deleted in a future version. {@link #TRUST_CUSTOM_CA_SIGNED_CERTIFICATES}. */ TRUST_SIGNED_CERTIFICATES: function( config, onSuccess, onFailure ) { - console.log("`TRUST_SIGNED_CERTIFICATES` has been deprecated as option and will be removed in a future version of " + + console.warn('`TRUST_SIGNED_CERTIFICATES` has been deprecated as option and will be removed in a future version of ' + "the driver. Please use `TRUST_CUSTOM_CA_SIGNED_CERTIFICATES` instead."); return TrustStrategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES(config, onSuccess, onFailure); }, @@ -172,7 +172,7 @@ const TrustStrategy = { * @deprecated in 1.1 in favour of {@link #TRUST_ALL_CERTIFICATES}. Will be deleted in a future version. */ TRUST_ON_FIRST_USE : function( config, onSuccess, onFailure ) { - console.log("`TRUST_ON_FIRST_USE` has been deprecated as option and will be removed in a future version of " + + console.warn('`TRUST_ON_FIRST_USE` has been deprecated as option and will be removed in a future version of ' + "the driver. Please use `TRUST_ALL_CERTIFICATES` instead."); let tlsOpts = { diff --git a/src/v1/internal/pool-config.js b/src/v1/internal/pool-config.js new file mode 100644 index 000000000..a5286b285 --- /dev/null +++ b/src/v1/internal/pool-config.js @@ -0,0 +1,69 @@ +/** + * Copyright (c) 2002-2017 "Neo Technology,"," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * 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. + */ + +const DEFAULT_MAX_SIZE = 50; +const DEFAULT_ACQUISITION_TIMEOUT = 60000; + +export default class PoolConfig { + + constructor(maxSize, acquisitionTimeout) { + this.maxSize = valueOrDefault(maxSize, DEFAULT_MAX_SIZE); + this.acquisitionTimeout = valueOrDefault(acquisitionTimeout, DEFAULT_ACQUISITION_TIMEOUT); + } + + static defaultConfig() { + return new PoolConfig(DEFAULT_MAX_SIZE, DEFAULT_ACQUISITION_TIMEOUT); + } + + static fromDriverConfig(config) { + const maxIdleSizeConfigured = isConfigured(config.connectionPoolSize); + const maxSizeConfigured = isConfigured(config.maxConnectionPoolSize); + + let maxSize; + + if (maxSizeConfigured) { + // correct size setting is set - use it's value + maxSize = config.maxConnectionPoolSize; + } else if (maxIdleSizeConfigured) { + // deprecated size setting is set - use it's value + console.warn('WARNING: neo4j-driver setting "connectionPoolSize" is deprecated, please use "maxConnectionPoolSize" instead'); + maxSize = config.connectionPoolSize; + } else { + maxSize = DEFAULT_MAX_SIZE; + } + + const acquisitionTimeoutConfigured = isConfigured(config.connectionAcquisitionTimeout); + const acquisitionTimeout = acquisitionTimeoutConfigured ? config.connectionAcquisitionTimeout : DEFAULT_ACQUISITION_TIMEOUT; + + return new PoolConfig(maxSize, acquisitionTimeout); + } +} + +function valueOrDefault(value, defaultValue) { + return value === 0 || value ? value : defaultValue; +} + +function isConfigured(value) { + return value === 0 || value; +} + +export { + DEFAULT_MAX_SIZE, + DEFAULT_ACQUISITION_TIMEOUT +}; diff --git a/src/v1/internal/pool.js b/src/v1/internal/pool.js index 7586f36fd..355af6d6c 100644 --- a/src/v1/internal/pool.js +++ b/src/v1/internal/pool.js @@ -17,27 +17,25 @@ * limitations under the License. */ -import { newError } from "../error"; -import { promiseOrTimeout } from "./util"; +import {promiseOrTimeout} from './util'; +import PoolConfig from './pool-config'; class Pool { /** - * @param create an allocation function that creates a new resource. It's given + * @param {function} create an allocation function that creates a new resource. It's given * a single argument, a function that will return the resource to * the pool if invoked, which is meant to be called on .dispose * or .close or whatever mechanism the resource uses to finalize. - * @param destroy called with the resource when it is evicted from this pool - * @param validate called at various times (like when an instance is acquired and + * @param {function} destroy called with the resource when it is evicted from this pool + * @param {function} validate called at various times (like when an instance is acquired and * when it is returned). If this returns false, the resource will * be evicted - * @param maxIdle the max number of resources that are allowed idle in the pool at - * any time. If this threshold is exceeded, resources will be evicted. + * @param {PoolConfig} config configuration for the new driver. */ - constructor(create, destroy=(()=>true), validate=(()=>true), config={}) { + constructor(create, destroy = (() => true), validate = (() => true), config = PoolConfig.defaultConfig()) { this._create = create; this._destroy = destroy; this._validate = validate; - this._maxIdleSize = config.maxIdleSize; this._maxSize = config.maxSize; this._acquisitionTimeout = config.acquisitionTimeout; this._pools = {}; @@ -151,7 +149,7 @@ class Pool { if (pool) { // there exist idle connections for the given key - if (pool.length >= this._maxIdleSize || !this._validate(resource)) { + if (!this._validate(resource)) { this._destroy(resource); } else { pool.push(resource); diff --git a/test/internal/pool-config.test.js b/test/internal/pool-config.test.js new file mode 100644 index 000000000..e8ea5ecf5 --- /dev/null +++ b/test/internal/pool-config.test.js @@ -0,0 +1,110 @@ +/** + * Copyright (c) 2002-2017 "Neo Technology,"," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * 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 PoolConfig, {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_MAX_SIZE} from '../../src/v1/internal/pool-config'; + +describe('PoolConfig', () => { + + let originalConsoleWarn; + + beforeAll(() => { + originalConsoleWarn = console.warn; + console.warn = () => { + }; + }); + + afterAll(() => { + console.warn = originalConsoleWarn; + }); + + it('should respect zero values', () => { + const config = new PoolConfig(0, 0, 0); + + expect(config.maxSize).toEqual(0); + expect(config.acquisitionTimeout).toEqual(0); + }); + + it('should expose default config', () => { + const config = PoolConfig.defaultConfig(); + + expect(config.maxSize).toEqual(DEFAULT_MAX_SIZE); + expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT); + }); + + it('should convert from empty driver config', () => { + const driverConfig = {}; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(DEFAULT_MAX_SIZE); + expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT); + }); + + it('should convert from full driver config', () => { + const driverConfig = { + maxConnectionPoolSize: 42, + connectionAcquisitionTimeout: 4242 + }; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(42); + expect(config.acquisitionTimeout).toEqual(4242); + }); + + it('should convert from driver config with both connectionPoolSize and maxConnectionPoolSize', () => { + const driverConfig = { + connectionPoolSize: 42, + maxConnectionPoolSize: 4242 + }; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(4242); + expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT); + }); + + it('should convert from driver config without connectionPoolSize and maxConnectionPoolSize', () => { + const driverConfig = { + connectionAcquisitionTimeout: 42 + }; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(DEFAULT_MAX_SIZE); + expect(config.acquisitionTimeout).toEqual(42); + }); + + it('should convert from driver config with only connectionPoolSize', () => { + const driverConfig = { + connectionPoolSize: 42 + }; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(42); + expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT); + }); + + it('should convert from driver config with only maxConnectionPoolSize', () => { + const driverConfig = { + maxConnectionPoolSize: 42 + }; + const config = PoolConfig.fromDriverConfig(driverConfig); + + expect(config.maxSize).toEqual(42); + expect(config.acquisitionTimeout).toEqual(DEFAULT_ACQUISITION_TIMEOUT); + }); + +}); diff --git a/test/internal/pool.test.js b/test/internal/pool.test.js index eb8272b1d..2c21218e7 100644 --- a/test/internal/pool.test.js +++ b/test/internal/pool.test.js @@ -18,6 +18,7 @@ */ import Pool from '../../src/v1/internal/pool'; +import PoolConfig from '../../src/v1/internal/pool-config'; describe('Pool', () => { @@ -103,44 +104,6 @@ describe('Pool', () => { }); }); - it('frees if pool reaches max size', (done) => { - // Given a pool that tracks destroyed resources - let counter = 0; - let destroyed = []; - const key = 'bolt://localhost:7687'; - const pool = new Pool( - (url, release) => new Resource(url, counter++, release), - resource => { - destroyed.push(resource); - }, - resource => true, - { - maxIdleSize: 2 - } - ); - - // When - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); - const p2 = pool.acquire(key); - - // Then - Promise.all([ p0, p1, p2 ]).then(values => { - const r0 = values[0]; - const r1 = values[1]; - const r2 = values[2]; - - r0.close(); - r1.close(); - r2.close(); - - expect(destroyed.length).toBe(1); - expect(destroyed[0].id).toBe(r2.id); - - done(); - }); - }); - it('frees if validate returns false', (done) => { // Given a pool that allocates let counter = 0; @@ -152,9 +115,7 @@ describe('Pool', () => { destroyed.push(resource); }, resource => false, - { - maxIdleSize: 1000 - } + new PoolConfig(1000, 60000) ); // When @@ -440,10 +401,7 @@ describe('Pool', () => { (url, release) => new Resource(url, counter++, release), resource => {}, resource => true, - { - maxSize: 2, - acquisitionTimeout: 5000 - } + new PoolConfig(2, 5000) ); const p0 = pool.acquire(key); @@ -473,10 +431,7 @@ describe('Pool', () => { (url, release) => new Resource(url, counter++, release), resource => {}, resource => true, - { - maxSize: 2, - acquisitionTimeout: 1000 - } + new PoolConfig(2, 1000) ); const p0 = pool.acquire(key); diff --git a/test/internal/shared-neo4j.js b/test/internal/shared-neo4j.js index 17148d7ae..6cce31f66 100644 --- a/test/internal/shared-neo4j.js +++ b/test/internal/shared-neo4j.js @@ -95,7 +95,7 @@ const password = 'password'; const authToken = neo4j.auth.basic(username, password); const neoCtrlVersionParam = '-e'; -const defaultNeo4jVersion = '3.2.0'; +const defaultNeo4jVersion = '3.2.5'; const defaultNeoCtrlArgs = `${neoCtrlVersionParam} ${defaultNeo4jVersion}`; function neo4jCertPath(dir) { diff --git a/test/v1/stress.test.js b/test/v1/stress.test.js index 29dddfc7e..cda2fff36 100644 --- a/test/v1/stress.test.js +++ b/test/v1/stress.test.js @@ -30,7 +30,7 @@ describe('stress tests', () => { fast: { commandsCount: 5000, parallelism: 8, - maxRunTimeMs: 120000 // 2 minutes + maxRunTimeMs: 180000 // 3 minutes }, extended: { commandsCount: 2000000, diff --git a/types/v1/driver.d.ts b/types/v1/driver.d.ts index 0bc532f57..57f393c6b 100644 --- a/types/v1/driver.d.ts +++ b/types/v1/driver.d.ts @@ -45,7 +45,11 @@ declare interface Config { trust?: TrustStrategy; trustedCertificates?: string[]; knownHosts?: string; + /** + * @deprecated use {@link maxConnectionPoolSize} instead. + */ connectionPoolSize?: number; + maxConnectionPoolSize?: number; maxTransactionRetryTime?: number; loadBalancingStrategy?: LoadBalancingStrategy; maxConnectionLifetime?: number;