Skip to content

Commit 7dbe303

Browse files
committed
Make LoadBalancer purge connections
Purging moved from `RoutingDriver` to the `LoadBalancer` to keep one place where we remove address from the routing table and purge all connections to it.
1 parent ad40851 commit 7dbe303

File tree

3 files changed

+32
-7
lines changed

3 files changed

+32
-7
lines changed

src/v1/internal/connection-providers.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export class LoadBalancer extends ConnectionProvider {
6767

6868
forget(address) {
6969
this._routingTable.forget(address);
70+
this._connectionPool.purge(address);
7071
}
7172

7273
forgetWriter(address) {
@@ -115,6 +116,7 @@ export class LoadBalancer extends ConnectionProvider {
115116
});
116117
}, Promise.resolve(null));
117118

119+
// todo: who removes the final router when all routers fail?
118120
return refreshedTablePromise.then(newRoutingTable => {
119121
if (newRoutingTable && !newRoutingTable.writers.isEmpty()) {
120122
this._updateRoutingTable(newRoutingTable);

src/v1/routing-driver.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ class RoutingDriver extends Driver {
3939
return new RoutingSession(connectionPromise, (error, conn) => {
4040
if (error.code === SERVICE_UNAVAILABLE || error.code === SESSION_EXPIRED) {
4141
if (conn) {
42-
this._forget(conn.url)
42+
this._connectionProvider.forget(conn.url);
4343
} else {
4444
connectionPromise.then((conn) => {
45-
this._forget(conn.url);
45+
this._connectionProvider.forget(conn.url);
4646
}).catch(() => {/*ignore*/});
4747
}
4848
return error;
@@ -63,11 +63,6 @@ class RoutingDriver extends Driver {
6363
});
6464
}
6565

66-
_forget(url) {
67-
this._connectionProvider.forget(url);
68-
this._pool.purge(url);
69-
}
70-
7166
static _validateConfig(config) {
7267
if(config.trust === 'TRUST_ON_FIRST_USE') {
7368
throw newError('The chosen trust mode is not compatible with a routing driver');

test/internal/connection-providers.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,28 @@ describe('LoadBalancer', () => {
7777
);
7878
});
7979

80+
it('purges connections when address is forgotten', () => {
81+
const pool = newPool();
82+
83+
pool.acquire('server-1');
84+
pool.acquire('server-3');
85+
pool.acquire('server-5');
86+
expectPoolToContain(pool, ['server-1', 'server-3', 'server-5']);
87+
88+
const loadBalancer = newLoadBalancer(
89+
['server-1', 'server-2'],
90+
['server-3', 'server-2'],
91+
['server-2', 'server-4'],
92+
pool
93+
);
94+
95+
loadBalancer.forget('server-1');
96+
loadBalancer.forget('server-5');
97+
98+
expectPoolToContain(pool, ['server-3']);
99+
expectPoolToNotContain(pool, ['server-1', 'server-5']);
100+
});
101+
80102
it('can forget writer address', () => {
81103
const loadBalancer = newLoadBalancer(
82104
['server-1', 'server-2'],
@@ -550,6 +572,12 @@ function expectRoutingTable(loadBalancer, routers, readers, writers) {
550572
expect(loadBalancer._routingTable.writers.toArray()).toEqual(writers);
551573
}
552574

575+
function expectPoolToContain(pool, addresses) {
576+
addresses.forEach(address => {
577+
expect(pool.has(address)).toBeTruthy();
578+
});
579+
}
580+
553581
function expectPoolToNotContain(pool, addresses) {
554582
addresses.forEach(address => {
555583
expect(pool.has(address)).toBeFalsy();

0 commit comments

Comments
 (0)