diff --git a/src/v1/index.js b/src/v1/index.js index 6103b7b23..948259443 100644 --- a/src/v1/index.js +++ b/src/v1/index.js @@ -132,7 +132,7 @@ function driver(url, authToken, config = {}) { return new RoutingDriver(parseUrl(url), routingContext, USER_AGENT, authToken, config); } else if (scheme === 'bolt://') { if (!isEmptyObjectOrNull(routingContext)) { - throw new Error(`Routing parameters are not supported with scheme 'bolt'. Given URL: '${url}'`); + throw new Error(`Parameters are not supported with scheme 'bolt'. Given URL: '${url}'`); } return new Driver(parseUrl(url), USER_AGENT, authToken, config); } else { diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index c3aff54bc..b9b633011 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import {newError, SERVICE_UNAVAILABLE} from '../error'; +import {newError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../error'; import {READ, WRITE} from '../driver'; import Session from '../session'; import RoundRobinArray from './round-robin-array'; @@ -70,16 +70,17 @@ export class LoadBalancer extends ConnectionProvider { this._connectionPool = connectionPool; this._driverOnErrorCallback = driverOnErrorCallback; this._hostNameResolver = LoadBalancer._createHostNameResolver(); + this._useSeedRouter = false; } - acquireConnection(mode) { - const connectionPromise = this._freshRoutingTable().then(routingTable => { - if (mode === READ) { + acquireConnection(accessMode) { + const connectionPromise = this._freshRoutingTable(accessMode).then(routingTable => { + if (accessMode === READ) { return this._acquireConnectionToServer(routingTable.readers, 'read'); - } else if (mode === WRITE) { + } else if (accessMode === WRITE) { return this._acquireConnectionToServer(routingTable.writers, 'write'); } else { - throw newError('Illegal mode ' + mode); + throw newError('Illegal mode ' + accessMode); } }); return this._withAdditionalOnErrorCallback(connectionPromise, this._driverOnErrorCallback); @@ -97,15 +98,17 @@ export class LoadBalancer extends ConnectionProvider { _acquireConnectionToServer(serversRoundRobinArray, serverName) { const address = serversRoundRobinArray.next(); if (!address) { - return Promise.reject(newError('No ' + serverName + ' servers available', SERVICE_UNAVAILABLE)); + return Promise.reject(newError( + `Failed to obtain connection towards ${serverName} server. Known routing table is: ${this._routingTable}`, + SESSION_EXPIRED)); } return this._connectionPool.acquire(address); } - _freshRoutingTable() { + _freshRoutingTable(accessMode) { const currentRoutingTable = this._routingTable; - if (!currentRoutingTable.isStale()) { + if (!currentRoutingTable.isStaleFor(accessMode)) { return Promise.resolve(currentRoutingTable); } return this._refreshRoutingTable(currentRoutingTable); @@ -114,48 +117,73 @@ export class LoadBalancer extends ConnectionProvider { _refreshRoutingTable(currentRoutingTable) { const knownRouters = currentRoutingTable.routers.toArray(); - return this._fetchNewRoutingTable(knownRouters, currentRoutingTable).then(newRoutingTable => { - if (LoadBalancer._isValidRoutingTable(newRoutingTable)) { - // one of the known routers returned a valid routing table - use it + if (this._useSeedRouter) { + return this._fetchRoutingTableFromSeedRouterFallbackToKnownRouters(knownRouters, currentRoutingTable); + } + return this._fetchRoutingTableFromKnownRoutersFallbackToSeedRouter(knownRouters, currentRoutingTable); + } + + _fetchRoutingTableFromSeedRouterFallbackToKnownRouters(knownRouters, currentRoutingTable) { + // we start with seed router, no routers were probed before + const seenRouters = []; + return this._fetchRoutingTableUsingSeedRouter(seenRouters, this._seedRouter).then(newRoutingTable => { + if (newRoutingTable) { + this._useSeedRouter = false; return newRoutingTable; } - if (!newRoutingTable) { - // returned routing table was undefined, this means a connection error happened and the last known - // router did not return a valid routing table, so we need to forget it - const lastRouterIndex = knownRouters.length - 1; - LoadBalancer._forgetRouter(currentRoutingTable, knownRouters, lastRouterIndex); + // seed router did not return a valid routing table - try to use other known routers + return this._fetchRoutingTableUsingKnownRouters(knownRouters, currentRoutingTable); + }).then(newRoutingTable => { + this._applyRoutingTableIfPossible(newRoutingTable); + return newRoutingTable; + }); + } + + _fetchRoutingTableFromKnownRoutersFallbackToSeedRouter(knownRouters, currentRoutingTable) { + return this._fetchRoutingTableUsingKnownRouters(knownRouters, currentRoutingTable).then(newRoutingTable => { + if (newRoutingTable) { + return newRoutingTable; } // none of the known routers returned a valid routing table - try to use seed router address for rediscovery - return this._fetchNewRoutingTableUsingSeedRouterAddress(knownRouters, this._seedRouter); + return this._fetchRoutingTableUsingSeedRouter(knownRouters, this._seedRouter); }).then(newRoutingTable => { - if (LoadBalancer._isValidRoutingTable(newRoutingTable)) { - this._updateRoutingTable(newRoutingTable); + this._applyRoutingTableIfPossible(newRoutingTable); + return newRoutingTable; + }); + } + + _fetchRoutingTableUsingKnownRouters(knownRouters, currentRoutingTable) { + return this._fetchRoutingTable(knownRouters, currentRoutingTable).then(newRoutingTable => { + if (newRoutingTable) { + // one of the known routers returned a valid routing table - use it return newRoutingTable; } - // none of the existing routers returned valid routing table, throw exception - throw newError('Could not perform discovery. No routing servers available.', SERVICE_UNAVAILABLE); + // returned routing table was undefined, this means a connection error happened and the last known + // router did not return a valid routing table, so we need to forget it + const lastRouterIndex = knownRouters.length - 1; + LoadBalancer._forgetRouter(currentRoutingTable, knownRouters, lastRouterIndex); + + return null; }); } - _fetchNewRoutingTableUsingSeedRouterAddress(knownRouters, seedRouter) { + _fetchRoutingTableUsingSeedRouter(seenRouters, seedRouter) { return this._hostNameResolver.resolve(seedRouter).then(resolvedRouterAddresses => { // filter out all addresses that we've already tried - const newAddresses = resolvedRouterAddresses.filter(address => knownRouters.indexOf(address) < 0); - return this._fetchNewRoutingTable(newAddresses, null); + const newAddresses = resolvedRouterAddresses.filter(address => seenRouters.indexOf(address) < 0); + return this._fetchRoutingTable(newAddresses, null); }); } - _fetchNewRoutingTable(routerAddresses, routingTable) { + _fetchRoutingTable(routerAddresses, routingTable) { return routerAddresses.reduce((refreshedTablePromise, currentRouter, currentIndex) => { return refreshedTablePromise.then(newRoutingTable => { if (newRoutingTable) { - if (!newRoutingTable.writers.isEmpty()) { - // valid routing table was fetched - just return it, try next router otherwise - return newRoutingTable; - } + // valid routing table was fetched - just return it, try next router otherwise + return newRoutingTable; } else { // returned routing table was undefined, this means a connection error happened and we need to forget the // previous router and try the next one @@ -179,6 +207,23 @@ export class LoadBalancer extends ConnectionProvider { return new Session(READ, connectionProvider); } + _applyRoutingTableIfPossible(newRoutingTable) { + if (!newRoutingTable) { + // none of routing servers returned valid routing table, throw exception + throw newError( + `Could not perform discovery. No routing servers available. Known routing table: ${this._routingTable}`, + SERVICE_UNAVAILABLE); + } + + if (newRoutingTable.writers.isEmpty()) { + // use seed router next time. this is important when cluster is partitioned. it tries to make sure driver + // does not always get routing table without writers because it talks exclusively to a minority partition + this._useSeedRouter = true; + } + + this._updateRoutingTable(newRoutingTable); + } + _updateRoutingTable(newRoutingTable) { const currentRoutingTable = this._routingTable; @@ -190,10 +235,6 @@ export class LoadBalancer extends ConnectionProvider { this._routingTable = newRoutingTable; } - static _isValidRoutingTable(routingTable) { - return routingTable && !routingTable.writers.isEmpty(); - } - static _forgetRouter(routingTable, routersArray, routerIndex) { const address = routersArray[routerIndex]; if (routingTable && address) { diff --git a/src/v1/internal/round-robin-array.js b/src/v1/internal/round-robin-array.js index d9ef40432..fa8e0ca87 100644 --- a/src/v1/internal/round-robin-array.js +++ b/src/v1/internal/round-robin-array.js @@ -59,4 +59,8 @@ export default class RoundRobinArray { remove(item) { this._items = this._items.filter(element => element !== item); } + + toString() { + return JSON.stringify(this._items); + } } diff --git a/src/v1/internal/routing-table.js b/src/v1/internal/routing-table.js index b1cb0a888..34d180006 100644 --- a/src/v1/internal/routing-table.js +++ b/src/v1/internal/routing-table.js @@ -18,6 +18,7 @@ */ import {int} from '../integer'; import RoundRobinArray from './round-robin-array'; +import {READ, WRITE} from '../driver'; const MIN_ROUTERS = 1; @@ -53,14 +54,28 @@ export default class RoutingTable { return Array.from(oldServers); } - isStale() { + /** + * Check if this routing table is fresh to perform the required operation. + * @param {string} accessMode the type of operation. Allowed values are {@link READ} and {@link WRITE}. + * @return {boolean} true when this table contains servers to serve the required operation, + * false otherwise. + */ + isStaleFor(accessMode) { return this.expirationTime.lessThan(Date.now()) || this.routers.size() < MIN_ROUTERS || - this.readers.isEmpty() || - this.writers.isEmpty(); + accessMode === READ && this.readers.isEmpty() || + accessMode === WRITE && this.writers.isEmpty(); } _allServers() { return [...this.routers.toArray(), ...this.readers.toArray(), ...this.writers.toArray()]; } + + toString() { + return `RoutingTable[` + + `expirationTime=${this.expirationTime}, ` + + `routers=${this.routers}, ` + + `readers=${this.readers}, ` + + `writers=${this.writers}]`; + } } diff --git a/test/internal/connection-providers.test.js b/test/internal/connection-providers.test.js index c7b35d33b..14e291f17 100644 --- a/test/internal/connection-providers.test.js +++ b/test/internal/connection-providers.test.js @@ -19,7 +19,7 @@ import {READ, WRITE} from '../../src/v1/driver'; import Integer, {int} from '../../src/v1/integer'; -import {SERVICE_UNAVAILABLE} from '../../src/v1/error'; +import {SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../../src/v1/error'; import RoutingTable from '../../src/v1/internal/routing-table'; import RoundRobinArray from '../../src/v1/internal/round-robin-array'; import {DirectConnectionProvider, LoadBalancer} from '../../src/v1/internal/connection-providers'; @@ -291,7 +291,7 @@ describe('LoadBalancer', () => { }); }); - it('refreshes stale routing table to get write connection', done => { + it('refreshes stale routing table to get write connection when one router fails', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], @@ -323,27 +323,22 @@ describe('LoadBalancer', () => { }); }); - it('refreshes stale routing table to get read connection when one router returns illegal response', done => { + it('refreshes routing table without readers to get read connection', done => { const pool = newPool(); - const newIllegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); - const newLegalRoutingTable = newRoutingTable( + const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C', 'server-D'], ['server-E', 'server-F'] ); const loadBalancer = newLoadBalancer( ['server-1', 'server-2'], + [], // no readers ['server-3', 'server-4'], - ['server-5', 'server-6'], pool, - int(0), // expired routing table + Integer.MAX_VALUE, { - 'server-1': newIllegalRoutingTable, - 'server-2': newLegalRoutingTable, + 'server-1': null, // returns no routing table + 'server-2': updatedRoutingTable, } ); @@ -360,14 +355,9 @@ describe('LoadBalancer', () => { }); }); - it('refreshes stale routing table to get write connection when one router returns illegal response', done => { + it('refreshes routing table without writers to get write connection', done => { const pool = newPool(); - const newIllegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); - const newLegalRoutingTable = newRoutingTable( + const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C', 'server-D'], ['server-E', 'server-F'] @@ -375,12 +365,12 @@ describe('LoadBalancer', () => { const loadBalancer = newLoadBalancer( ['server-1', 'server-2'], ['server-3', 'server-4'], - ['server-5', 'server-6'], + [], // no writers pool, int(0), // expired routing table { - 'server-1': newIllegalRoutingTable, - 'server-2': newLegalRoutingTable, + 'server-1': null, // returns no routing table + 'server-2': updatedRoutingTable, } ); @@ -435,11 +425,11 @@ describe('LoadBalancer', () => { }); }); - it('throws when all routers return illegal routing tables while getting read connection', done => { - const newIllegalRoutingTable = newRoutingTable( + it('throws when all routers return routing tables without readers while getting read connection', done => { + const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped + [], // no readers - table can't satisfy connection requirement + ['server-C', 'server-D'] ); const loadBalancer = newLoadBalancer( ['server-1', 'server-2'], @@ -448,22 +438,22 @@ describe('LoadBalancer', () => { newPool(), int(0), // expired routing table { - 'server-1': newIllegalRoutingTable, - 'server-2': newIllegalRoutingTable + 'server-1': updatedRoutingTable, + 'server-2': updatedRoutingTable } ); loadBalancer.acquireConnection(READ).catch(error => { - expect(error.code).toEqual(SERVICE_UNAVAILABLE); + expect(error.code).toEqual(SESSION_EXPIRED); done(); }); }); - it('throws when all routers return illegal routing tables while getting write connection', done => { - const newIllegalRoutingTable = newRoutingTable( + it('throws when all routers return routing tables without writers while getting write connection', done => { + const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped + [] // no writers - table can't satisfy connection requirement ); const loadBalancer = newLoadBalancer( ['server-1', 'server-2'], @@ -472,13 +462,13 @@ describe('LoadBalancer', () => { newPool(), int(0), // expired routing table { - 'server-1': newIllegalRoutingTable, - 'server-2': newIllegalRoutingTable + 'server-1': updatedRoutingTable, + 'server-2': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).catch(error => { - expect(error.code).toEqual(SERVICE_UNAVAILABLE); + expect(error.code).toEqual(SESSION_EXPIRED); done(); }); }); @@ -582,12 +572,7 @@ describe('LoadBalancer', () => { }); }); - it('uses seed router address when all existing routers failed', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); + it('uses seed router address when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B', 'server-C'], ['server-D', 'server-E'], @@ -602,7 +587,7 @@ describe('LoadBalancer', () => { int(0), // expired routing table { 'server-1': null, // returns no routing table - 'server-2': illegalRoutingTable, + 'server-2': null, // returns no routing table 'server-3': null, // returns no routing table 'server-0': updatedRoutingTable } @@ -624,12 +609,7 @@ describe('LoadBalancer', () => { }); }); - it('uses resolved seed router address when all existing routers failed', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A'], - ['server-B'], - [] // no writers - table is illegal and should be skipped - ); + it('uses resolved seed router address when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C', 'server-D'], @@ -643,8 +623,8 @@ describe('LoadBalancer', () => { ['server-6', 'server-7'], int(0), // expired routing table { - 'server-1': illegalRoutingTable, - 'server-2': illegalRoutingTable, + 'server-1': null, // returns no routing table + 'server-2': null, // returns no routing table 'server-3': null, // returns no routing table 'server-01': updatedRoutingTable } @@ -666,12 +646,7 @@ describe('LoadBalancer', () => { }); }); - it('uses resolved seed router address that returns correct routing table when all existing routers failed', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); + it('uses resolved seed router address that returns correct routing table when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C'], @@ -685,9 +660,9 @@ describe('LoadBalancer', () => { ['server-3'], int(0), // expired routing table { - 'server-1': illegalRoutingTable, + 'server-1': null, // returns no routing table 'server-01': null, // returns no routing table - 'server-02': illegalRoutingTable, + 'server-02': null, // returns no routing table 'server-03': updatedRoutingTable } ); @@ -708,9 +683,7 @@ describe('LoadBalancer', () => { }); }); - it('fails when existing routers fail and seed router returns an invalid routing table', done => { - const emptyIllegalRoutingTable = newRoutingTable([], [], []); - + it('fails when both existing routers and seed router fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( 'server-0', ['server-0'], // seed router address resolves just to itself ['server-1', 'server-2', 'server-3'], @@ -718,10 +691,10 @@ describe('LoadBalancer', () => { ['server-6'], int(0), // expired routing table { - 'server-1': emptyIllegalRoutingTable, + 'server-1': null, // returns no routing table 'server-2': null, // returns no routing table 'server-3': null, // returns no routing table - 'server-0': emptyIllegalRoutingTable + 'server-0': null // returns no routing table } ); @@ -729,7 +702,7 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, - ['server-1'], // only server-1 is in the table, it returned a routing table which turned out to be invalid + [], // all routers were forgotten because they failed ['server-4', 'server-5'], ['server-6'], ); @@ -738,7 +711,7 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, - ['server-1'], // only server-1 is in the table, it returned a routing table which turned out to be invalid + [], // all routers were forgotten because they failed ['server-4', 'server-5'], ['server-6'], ); @@ -748,13 +721,7 @@ describe('LoadBalancer', () => { }); }); - it('fails when existing routers fail and resolved seed router returns an invalid routing table', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); - + it('fails when both existing routers and resolved seed router fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( 'server-0', ['server-01'], // seed router address resolves to a different one ['server-1', 'server-2'], @@ -763,7 +730,7 @@ describe('LoadBalancer', () => { int(0), // expired routing table { 'server-1': null, // returns no routing table - 'server-2': illegalRoutingTable, + 'server-2': null, // returns no routing table 'server-01': null // returns no routing table } ); @@ -772,7 +739,7 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, - ['server-2'], // only server-2 is in the table, it returned a routing table which turned out to be invalid + [], // all routers were forgotten because they failed ['server-3'], ['server-4'], ); @@ -781,7 +748,7 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, - ['server-2'], // only server-2 is in the table, it returned a routing table which turned out to be invalid + [], // all routers were forgotten because they failed ['server-3'], ['server-4'], ); @@ -791,13 +758,7 @@ describe('LoadBalancer', () => { }); }); - it('fails when existing routers fail and all resolved seed routers return an invalid routing table', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - [] // no writers - table is illegal and should be skipped - ); - + it('fails when both existing routers and all resolved seed routers fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( 'server-0', ['server-02', 'server-01'], // seed router address resolves to 2 different addresses ['server-1', 'server-2', 'server-3'], @@ -808,7 +769,7 @@ describe('LoadBalancer', () => { 'server-1': null, // returns no routing table 'server-2': null, // returns no routing table 'server-3': null, // returns no routing table - 'server-01': illegalRoutingTable, + 'server-01': null, // returns no routing table 'server-02': null // returns no routing table } ); @@ -904,12 +865,7 @@ describe('LoadBalancer', () => { }); }); - it('uses resolved seed router that returns correct routing table when no existing routers exist', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A'], - ['server-B'], - [] // no writers - table is illegal and should be skipped - ); + it('uses resolved seed router that returns routing table when no existing routers exist', done => { const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B', 'server-C'], ['server-D', 'server-E'], @@ -923,8 +879,8 @@ describe('LoadBalancer', () => { ['server-2', 'server-3'], Integer.MAX_VALUE, // not expired { - 'server-01': null, - 'server-02': illegalRoutingTable, + 'server-01': null, // returns no routing table + 'server-02': null, // returns no routing table 'server-03': updatedRoutingTable } ); @@ -946,11 +902,6 @@ describe('LoadBalancer', () => { }); it('ignores already probed routers after seed router resolution', done => { - const illegalRoutingTable = newRoutingTable( - ['server-A'], - ['server-B'], - [] // no writers - table is illegal and should be skipped - ); const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], ['server-C', 'server-D'], @@ -964,9 +915,9 @@ describe('LoadBalancer', () => { ['server-5', 'server-6'], int(0), // expired routing table { - 'server-1': null, - 'server-01': null, - 'server-2': illegalRoutingTable, + 'server-1': null, // returns no routing table + 'server-01': null, // returns no routing table + 'server-2': null, // returns no routing table 'server-02': updatedRoutingTable } ); @@ -996,11 +947,11 @@ describe('LoadBalancer', () => { }); }); - it('throws service unavailable when refreshed routing table has no readers', done => { + it('throws session expired when refreshed routing table has no readers', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( ['server-A', 'server-B'], - [], + [], // no readers ['server-C', 'server-D'] ); const loadBalancer = newLoadBalancer( @@ -1015,11 +966,89 @@ describe('LoadBalancer', () => { ); loadBalancer.acquireConnection(READ).catch(error => { - expect(error.code).toEqual(SERVICE_UNAVAILABLE); + expect(error.code).toEqual(SESSION_EXPIRED); done(); }); }); + it('throws session expired when refreshed routing table has no writers', done => { + const pool = newPool(); + const updatedRoutingTable = newRoutingTable( + ['server-A', 'server-B'], + ['server-C', 'server-D'], + [] // no writers + ); + const loadBalancer = newLoadBalancer( + ['server-1', 'server-2'], + ['server-3', 'server-4'], + ['server-5', 'server-6'], + pool, + int(0), // expired routing table + { + 'server-1': updatedRoutingTable, + } + ); + + loadBalancer.acquireConnection(WRITE).catch(error => { + expect(error.code).toEqual(SESSION_EXPIRED); + done(); + }); + }); + + it('should use resolved seed router after accepting table with no writers', done => { + const routingTable1 = newRoutingTable( + ['server-A', 'server-B'], + ['server-C', 'server-D'], + [] // no writers + ); + const routingTable2 = newRoutingTable( + ['server-AA', 'server-BB'], + ['server-CC', 'server-DD'], + ['server-EE'] + ); + + const loadBalancer = newLoadBalancerWithSeedRouter( + 'server-0', ['server-02', 'server-01'], // seed router address resolves to 2 different addresses + ['server-1'], + ['server-2', 'server-3'], + ['server-4', 'server-5'], + int(0), // expired routing table + { + 'server-1': routingTable1, + 'server-A': routingTable1, + 'server-B': routingTable1, + 'server-01': null, // returns no routing table + 'server-02': routingTable2 + } + ); + + loadBalancer.acquireConnection(READ).then(connection1 => { + expect(connection1.address).toEqual('server-C'); + + loadBalancer.acquireConnection(READ).then(connection2 => { + expect(connection2.address).toEqual('server-D'); + + expectRoutingTable(loadBalancer, + ['server-A', 'server-B'], + ['server-C', 'server-D'], + [] + ); + + loadBalancer.acquireConnection(WRITE).then(connection3 => { + expect(connection3.address).toEqual('server-EE'); + + expectRoutingTable(loadBalancer, + ['server-AA', 'server-BB'], + ['server-CC', 'server-DD'], + ['server-EE'] + ); + + done(); + }); + }); + }); + }); + }); function newDirectConnectionProvider(address, pool) { @@ -1062,12 +1091,12 @@ function newRoutingTable(routers, readers, writers, expirationTime = Integer.MAX } function setupLoadBalancerToRememberRouters(loadBalancer, routersArray) { - const originalFetch = loadBalancer._fetchNewRoutingTable.bind(loadBalancer); + const originalFetch = loadBalancer._fetchRoutingTable.bind(loadBalancer); const rememberingFetch = (routerAddresses, routingTable) => { routersArray.push(routerAddresses); return originalFetch(routerAddresses, routingTable); }; - loadBalancer._fetchNewRoutingTable = rememberingFetch; + loadBalancer._fetchRoutingTable = rememberingFetch; } function newPool() { diff --git a/test/internal/connector.test.js b/test/internal/connector.test.js index ba29ec2c4..6d9ba1091 100644 --- a/test/internal/connector.test.js +++ b/test/internal/connector.test.js @@ -132,9 +132,7 @@ describe('connector', () => { it('should notify when connection initialization fails', done => { const connection = connect('bolt://localhost:7474'); // wrong port - connection.initializationCompleted().then(() => { - console.log('THEN called: ', arguments) - }).catch(error => { + connection.initializationCompleted().catch(error => { expect(error).toBeDefined(); done(); }); diff --git a/test/internal/round-robin-array.test.js b/test/internal/round-robin-array.test.js index 1064b47d2..028bdc090 100644 --- a/test/internal/round-robin-array.test.js +++ b/test/internal/round-robin-array.test.js @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import RoundRobinArray from "../../lib/v1/internal/round-robin-array"; +import RoundRobinArray from '../../lib/v1/internal/round-robin-array'; describe('round-robin-array', () => { @@ -198,4 +198,9 @@ describe('round-robin-array', () => { expect(array.next()).toEqual(3); }); + it('should have correct toString ', () => { + const array = new RoundRobinArray([1, 2, 3]); + expect(array.toString()).toEqual('[1,2,3]'); + }); + }); diff --git a/test/internal/routing-table.test.js b/test/internal/routing-table.test.js index 8ea5f343f..fb64fd33c 100644 --- a/test/internal/routing-table.test.js +++ b/test/internal/routing-table.test.js @@ -19,42 +19,50 @@ import RoutingTable from '../../src/v1/internal/routing-table'; import RoundRobinArray from '../../src/v1/internal/round-robin-array'; import {int} from '../../src/v1/integer'; +import {READ, WRITE} from '../../src/v1/driver'; describe('routing-table', () => { it('should not be stale when has routers, readers, writers and future expiration date', () => { const table = createTable([1, 2], [3, 4], [5, 6], notExpired()); - expect(table.isStale()).toBeFalsy(); + expect(table.isStaleFor(READ)).toBeFalsy(); + expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should be stale when expiration date in the past', () => { const table = createTable([1, 2], [1, 2], [1, 2], expired()); - expect(table.isStale()).toBeTruthy(); + expect(table.isStaleFor(READ)).toBeTruthy(); + expect(table.isStaleFor(WRITE)).toBeTruthy(); }); it('should not be stale when has single router', () => { const table = createTable([1], [2, 3], [4, 5], notExpired()); - expect(table.isStale()).toBeFalsy(); + expect(table.isStaleFor(READ)).toBeFalsy(); + expect(table.isStaleFor(WRITE)).toBeFalsy(); }); - it('should be stale when no readers', () => { + it('should be stale for reads but not writes when no readers', () => { const table = createTable([1, 2], [], [3, 4], notExpired()); - expect(table.isStale()).toBeTruthy(); + expect(table.isStaleFor(READ)).toBeTruthy(); + expect(table.isStaleFor(WRITE)).toBeFalsy(); }); - it('should be stale when no writers', () => { + it('should be stale for writes but not reads when no writers', () => { const table = createTable([1, 2], [3, 4], [], notExpired()); - expect(table.isStale()).toBeTruthy(); + expect(table.isStaleFor(READ)).toBeFalsy(); + expect(table.isStaleFor(WRITE)).toBeTruthy(); }); it('should not be stale with single reader', () => { const table = createTable([1, 2], [3], [4, 5], notExpired()); - expect(table.isStale()).toBeFalsy(); + expect(table.isStaleFor(READ)).toBeFalsy(); + expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should not be stale with single writer', () => { const table = createTable([1, 2], [3, 4], [5], notExpired()); - expect(table.isStale()).toBeFalsy(); + expect(table.isStaleFor(READ)).toBeFalsy(); + expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should forget reader, writer but not router', () => { @@ -161,6 +169,11 @@ describe('routing-table', () => { expect(servers).toEqual([11, 22, 33, 44]); }); + it('should have correct toString', () => { + const table = createTable([1, 2], [3, 4], [5, 6], 42); + expect(table.toString()).toEqual('RoutingTable[expirationTime=42, routers=[1,2], readers=[3,4], writers=[5,6]]'); + }); + function expired() { return Date.now() - 3600; // expired an hour ago } diff --git a/test/resources/boltkit/dead_routing_server.script b/test/resources/boltkit/dead_routing_server.script new file mode 100644 index 000000000..2791eaea9 --- /dev/null +++ b/test/resources/boltkit/dead_routing_server.script @@ -0,0 +1,7 @@ +!: AUTO INIT +!: AUTO RESET +!: AUTO PULL_ALL + +C: RUN "CALL dbms.cluster.routing.getServers" {} +C: PULL_ALL +S: diff --git a/test/resources/boltkit/discover_no_writers.script b/test/resources/boltkit/discover_no_writers.script new file mode 100644 index 000000000..20f69dde4 --- /dev/null +++ b/test/resources/boltkit/discover_no_writers.script @@ -0,0 +1,9 @@ +!: AUTO INIT +!: AUTO RESET +!: AUTO PULL_ALL + +C: RUN "CALL dbms.cluster.routing.getServers" {} + PULL_ALL +S: SUCCESS {"fields": ["ttl", "servers"]} + RECORD [9223372036854775807, [{"addresses": [],"role": "WRITE"}, {"addresses": ["127.0.0.1:9002","127.0.0.1:9003"], "role": "READ"},{"addresses": ["127.0.0.1:9004","127.0.0.1:9005"], "role": "ROUTE"}]] + SUCCESS {} diff --git a/test/v1/routing.driver.boltkit.it.js b/test/v1/routing.driver.boltkit.it.js index c5f700284..39d0ed4c0 100644 --- a/test/v1/routing.driver.boltkit.it.js +++ b/test/v1/routing.driver.boltkit.it.js @@ -670,7 +670,7 @@ describe('routing driver', () => { // When const session = driver.session(neo4j.session.WRITE); session.run("MATCH (n) RETURN n.name").catch(err => { - expect(err.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE); + expect(err.code).toEqual(neo4j.error.SESSION_EXPIRED); driver.close(); seedServer.exit(code => { expect(code).toEqual(0); @@ -680,7 +680,7 @@ describe('routing driver', () => { }); }); - it('should try next router when no writers', done => { + it('should try next router when current router fails to return a routing table', done => { if (!boltkit.BoltKitSupport) { done(); return; @@ -688,9 +688,9 @@ describe('routing driver', () => { const kit = new boltkit.BoltKit(); const server1 = kit.start('./test/resources/boltkit/routing_table_with_zero_ttl.script', 9999); - const server2 = kit.start('./test/resources/boltkit/no_writers.script', 9091); - const server3 = kit.start('./test/resources/boltkit/no_writers.script', 9092); - const server4 = kit.start('./test/resources/boltkit/no_writers.script', 9093); + const server2 = kit.start('./test/resources/boltkit/dead_routing_server.script', 9091); + const server3 = kit.start('./test/resources/boltkit/dead_routing_server.script', 9092); + const server4 = kit.start('./test/resources/boltkit/dead_routing_server.script', 9093); kit.run(() => { const driver = newDriver('bolt+routing://127.0.0.1:9999'); @@ -708,7 +708,8 @@ describe('routing driver', () => { expect(result2.summary.server.address).toEqual('127.0.0.1:9999'); session2.close(); - memorizingRoutingTable.assertForgotRouters([]); + // returned routers failed to respond and should have been forgotten + memorizingRoutingTable.assertForgotRouters(['127.0.0.1:9091', '127.0.0.1:9092', '127.0.0.1:9093']); assertHasRouters(driver, ['127.0.0.1:9999']); driver.close(); @@ -1672,6 +1673,164 @@ describe('routing driver', () => { }); }); + it('should use routing table without writers for reads', done => { + const kit = new boltkit.BoltKit(); + const router = kit.start('./test/resources/boltkit/discover_no_writers.script', 9010); + const reader = kit.start('./test/resources/boltkit/read_server.script', 9002); + + kit.run(() => { + const driver = newDriver('bolt+routing://127.0.0.1:9010'); + + const session = driver.session(READ); + session.run('MATCH (n) RETURN n.name').then(result => { + session.close(() => { + expect(result.records.map(record => record.get(0))).toEqual(['Bob', 'Alice', 'Tina']); + + driver.close(); + + router.exit(code1 => { + reader.exit(code2 => { + expect(code1).toEqual(0); + expect(code2).toEqual(0); + done(); + }); + }); + }); + }); + }); + }); + + it('should serve reads but fail writes when no writers available', done => { + const kit = new boltkit.BoltKit(); + const router1 = kit.start('./test/resources/boltkit/discover_no_writers.script', 9010); + const router2 = kit.start('./test/resources/boltkit/discover_no_writers.script', 9004); + const reader = kit.start('./test/resources/boltkit/read_server.script', 9003); + + kit.run(() => { + const driver = newDriver('bolt+routing://127.0.0.1:9010'); + + const readSession = driver.session(); + + readSession.readTransaction(tx => tx.run('MATCH (n) RETURN n.name')).then(result => { + readSession.close(() => { + expect(result.records.map(record => record.get(0))).toEqual(['Bob', 'Alice', 'Tina']); + + const writeSession = driver.session(WRITE); + writeSession.run('CREATE (n {name:\'Bob\'})').catch(error => { + expect(error.code).toEqual(neo4j.error.SESSION_EXPIRED); + + driver.close(); + + router1.exit(code1 => { + router2.exit(code2 => { + reader.exit(code3 => { + expect(code1).toEqual(0); + expect(code2).toEqual(0); + expect(code3).toEqual(0); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + + it('should accept routing table without writers and then rediscover', done => { + const kit = new boltkit.BoltKit(); + + // first router does not have itself in the resulting routing table so connection + // towards it will be closed after rediscovery + const router1 = kit.start('./test/resources/boltkit/discover_no_writers.script', 9010); + let router2 = null; + const reader = kit.start('./test/resources/boltkit/read_server.script', 9003); + const writer = kit.start('./test/resources/boltkit/write_server.script', 9007); + + kit.run(() => { + const driver = newDriver('bolt+routing://127.0.0.1:9010'); + + const readSession = driver.session(); + + readSession.readTransaction(tx => tx.run('MATCH (n) RETURN n.name')).then(result => { + readSession.close(() => { + expect(result.records.map(record => record.get(0))).toEqual(['Bob', 'Alice', 'Tina']); + + // start another router which knows about writes, use same address as the initial router + router2 = kit.start('./test/resources/boltkit/acquire_endpoints.script', 9010); + kit.run(() => { + const writeSession = driver.session(WRITE); + writeSession.run('CREATE (n {name:\'Bob\'})').then(result => { + writeSession.close(() => { + expect(result.records).toEqual([]); + + driver.close(); + + router1.exit(code1 => { + router2.exit(code2 => { + reader.exit(code3 => { + writer.exit(code4 => { + expect(code1).toEqual(0); + expect(code2).toEqual(0); + expect(code3).toEqual(0); + expect(code4).toEqual(0); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + }); + }); + }); + + it('should use resolved seed router for discovery after accepting a table without writers', done => { + const kit = new boltkit.BoltKit(); + const seedRouter = kit.start('./test/resources/boltkit/no_writers.script', 9010); + const resolvedSeedRouter = kit.start('./test/resources/boltkit/acquire_endpoints.script', 9020); + const reader = kit.start('./test/resources/boltkit/read_server.script', 9005); + const writer = kit.start('./test/resources/boltkit/write_server.script', 9007); + + kit.run(() => { + const driver = newDriver('bolt+routing://127.0.0.1:9010'); + + const readSession = driver.session(READ); + readSession.run('MATCH (n) RETURN n.name').then(result => { + readSession.close(() => { + expect(result.records.map(record => record.get(0))).toEqual(['Bob', 'Alice', 'Tina']); + + setupFakeHostNameResolution(driver, '127.0.0.1:9010', ['127.0.0.1:9020']); + + const writeSession = driver.session(WRITE); + writeSession.run('CREATE (n {name:\'Bob\'})').then(result => { + writeSession.close(() => { + expect(result.records).toEqual([]); + + driver.close(); + + seedRouter.exit(code1 => { + resolvedSeedRouter.exit(code2 => { + reader.exit(code3 => { + writer.exit(code4 => { + expect(code1).toEqual(0); + expect(code2).toEqual(0); + expect(code3).toEqual(0); + expect(code4).toEqual(0); + done(); + }); + }); + }); + }); + }); + }); + }); + }); + }); + }); + function moveNextDateNow30SecondsForward() { const currentTime = Date.now(); hijackNextDateNowCall(currentTime + 30 * 1000 + 1);