Skip to content

Commit 6752c81

Browse files
vlad-dargelvlad-dargel
authored
Merge pull request #2806 from RedisInsight/feature/RI-5167-connection-retry
Feature/ri 5167 connection retry
2 parents 0d944e8 + cc3efc7 commit 6752c81

File tree

5 files changed

+174
-62
lines changed

5 files changed

+174
-62
lines changed

‎redisinsight/api/config/default.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export default {
7777
redis_clients: {
7878
idleSyncInterval: parseInt(process.env.CLIENTS_IDLE_SYNC_INTERVAL, 10) || 1000 * 60 * 60, // 1hr
7979
maxIdleThreshold: parseInt(process.env.CLIENTS_MAX_IDLE_THRESHOLD, 10) || 1000 * 60 * 60, // 1hr
80-
retryTimes: parseInt(process.env.CLIENTS_RETRY_TIMES, 10) || 5,
80+
retryTimes: parseInt(process.env.CLIENTS_RETRY_TIMES, 10) || 3,
8181
retryDelay: parseInt(process.env.CLIENTS_RETRY_DELAY, 10) || 500,
8282
maxRetriesPerRequest: parseInt(process.env.CLIENTS_MAX_RETRIES_PER_REQUEST, 10) || 1,
8383
},

‎redisinsight/api/src/constants/error-messages.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ export default {
1616
CONNECTION_TIMEOUT:
1717
'The connection has timed out, please check the connection details.',
1818
SERVER_CLOSED_CONNECTION: 'Server closed the connection.',
19+
UNABLE_TO_ESTABLISH_CONNECTION: 'Unable to establish connection.',
20+
RECONNECTING_TO_DATABASE: 'Reconnecting to the redis database.',
1921
AUTHENTICATION_FAILED: () => 'Failed to authenticate, please check the username or password.',
2022
INCORRECT_DATABASE_URL: (url) => `Could not connect to ${url}, please check the connection details.`,
2123
INCORRECT_CERTIFICATES: (url) => `Could not connect to ${url}, please check the CA or Client certificate.`,

‎redisinsight/api/src/modules/database/providers/database.factory.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class DatabaseFactory {
3838
context: ClientContext.Common,
3939
},
4040
database,
41-
{ useRetry: false },
41+
{ useRetry: true },
4242
);
4343

4444
if (await this.databaseInfoProvider.isSentinel(client)) {
@@ -109,7 +109,7 @@ export class DatabaseFactory {
109109
context: ClientContext.Common,
110110
},
111111
model,
112-
{ useRetry: false },
112+
{ useRetry: true },
113113
);
114114

115115
// todo: rethink
@@ -157,7 +157,7 @@ export class DatabaseFactory {
157157
context: ClientContext.Common,
158158
},
159159
model,
160-
{ useRetry: false },
160+
{ useRetry: true },
161161
);
162162

163163
model.connectionType = ConnectionType.SENTINEL;

‎redisinsight/api/src/modules/redis/redis-connection.factory.spec.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,21 @@ describe('RedisConnectionFactory', () => {
8787
process.nextTick(() => mockClient.emit('ready'));
8888
});
8989

90-
it('should fail to create standalone connection', (done) => {
90+
it('should successfully create standalone client even with error event emited', (done) => {
91+
service.createStandaloneConnection(mockClientMetadata, mockDatabaseWithTlsAuth, { useRetry: true })
92+
.then(checkClient(done, mockClient));
93+
process.nextTick(() => mockClient.emit('error', mockError));
94+
process.nextTick(() => mockClient.emit('ready'));
95+
});
96+
97+
it('should fail to create standalone with last error', (done) => {
9198
service.createStandaloneConnection(mockClientMetadata, mockDatabaseWithTlsAuth, {})
9299
.catch(checkError(done));
93100

101+
process.nextTick(() => mockClient.emit('error', new Error('1')));
102+
process.nextTick(() => mockClient.emit('error', new Error('2')));
94103
process.nextTick(() => mockClient.emit('error', mockError));
104+
process.nextTick(() => mockClient.emit('end'));
95105
});
96106

97107
it('should handle sync error during standalone client creation', (done) => {
@@ -113,11 +123,22 @@ describe('RedisConnectionFactory', () => {
113123
process.nextTick(() => mockCluster.emit('ready'));
114124
});
115125

116-
it('should fail to create cluster connection', (done) => {
126+
it('should successfully create cluster client and not fail even when error emited', (done) => {
127+
service.createClusterConnection(mockClientMetadata, mockClusterDatabaseWithTlsAuth, {})
128+
.then(checkClient(done, mockCluster));
129+
130+
process.nextTick(() => mockCluster.emit('error', mockError));
131+
process.nextTick(() => mockCluster.emit('ready'));
132+
});
133+
134+
it('should fail to create cluster connection with last error', (done) => {
117135
service.createClusterConnection(mockClientMetadata, mockClusterDatabaseWithTlsAuth, {})
118136
.catch(checkError(done));
119137

138+
process.nextTick(() => mockCluster.emit('error', new Error('1')));
139+
process.nextTick(() => mockCluster.emit('error', new Error('2')));
120140
process.nextTick(() => mockCluster.emit('error', mockError));
141+
process.nextTick(() => mockCluster.emit('end'));
121142
});
122143

123144
it('should handle sync error during cluster client creation', (done) => {
@@ -138,11 +159,22 @@ describe('RedisConnectionFactory', () => {
138159
process.nextTick(() => mockClient.emit('ready'));
139160
});
140161

141-
it('should fail to create sentinel connection', (done) => {
162+
it('should successfully create sentinel client and not fail even when error emited', (done) => {
163+
service.createSentinelConnection(mockClientMetadata, mockSentinelDatabaseWithTlsAuth, { useRetry: true })
164+
.then(checkClient(done, mockClient));
165+
166+
process.nextTick(() => mockClient.emit('error', mockError));
167+
process.nextTick(() => mockClient.emit('ready'));
168+
});
169+
170+
it('should fail to create sentinel connection with last error', (done) => {
142171
service.createSentinelConnection(mockClientMetadata, mockSentinelDatabaseWithTlsAuth, {})
143172
.catch(checkError(done));
144173

174+
process.nextTick(() => mockClient.emit('error', new Error('1')));
175+
process.nextTick(() => mockClient.emit('error', new Error('2')));
145176
process.nextTick(() => mockClient.emit('error', mockError));
177+
process.nextTick(() => mockClient.emit('end'));
146178
});
147179

148180
it('should handle sync error during sentinel client creation', (done) => {

‎redisinsight/api/src/modules/redis/redis-connection.factory.ts‎

Lines changed: 133 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Injectable, InternalServerErrorException, Logger } from '@nestjs/common
33
import { Database } from 'src/modules/database/models/database';
44
import apiConfig from 'src/utils/config';
55
import { ConnectionOptions } from 'tls';
6-
import { isEmpty, isNumber } from 'lodash';
6+
import { isEmpty, isNumber, set } from 'lodash';
77
import { cloneClassInstance, generateRedisConnectionName } from 'src/utils';
88
import { ConnectionType } from 'src/modules/database/entities/database.entity';
99
import { ClientMetadata } from 'src/common/models';
@@ -164,16 +164,21 @@ export class RedisConnectionFactory {
164164
options: IRedisConnectionOptions,
165165
): Promise<Redis> {
166166
let tnl;
167+
let connection: Redis;
167168

168169
try {
169170
const config = await this.getRedisOptions(clientMetadata, database, options);
171+
// cover cases when we are connecting to sentinel using standalone client to discover master groups
172+
const dbIndex = config.db > 0 && !database.sentinelMaster ? config.db : 0;
170173

171174
if (database.ssh) {
172175
tnl = await this.sshTunnelProvider.createTunnel(database);
173176
}
174177

175178
return await new Promise((resolve, reject) => {
176179
try {
180+
let lastError: Error;
181+
177182
if (tnl) {
178183
tnl.on('error', (error) => {
179184
reject(error);
@@ -187,31 +192,45 @@ export class RedisConnectionFactory {
187192
config.port = tnl.serverAddress.port;
188193
}
189194

190-
const connection = new Redis({
195+
connection = new Redis({
191196
...config,
192-
// cover cases when we are connecting to sentinel as to standalone to discover master groups
193-
db: config.db > 0 && !database.sentinelMaster ? config.db : 0,
197+
db: 0,
194198
});
195199
connection.on('error', (e): void => {
196200
this.logger.error('Failed connection to the redis database.', e);
197-
reject(e);
201+
lastError = e;
198202
});
199203
connection.on('end', (): void => {
200-
this.logger.error(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION);
201-
reject(new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
204+
this.logger.error(ERROR_MESSAGES.UNABLE_TO_ESTABLISH_CONNECTION, lastError);
205+
reject(lastError || new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
202206
});
203207
connection.on('ready', (): void => {
208+
lastError = null;
204209
this.logger.log('Successfully connected to the redis database');
205-
resolve(connection);
210+
211+
// manual switch to particular logical db
212+
// since ioredis doesn't handle "select" command error during connection
213+
if (dbIndex > 0) {
214+
connection.select(dbIndex)
215+
.then(() => {
216+
set(connection, ['options', 'db'], dbIndex);
217+
resolve(connection);
218+
})
219+
.catch(reject);
220+
} else {
221+
resolve(connection);
222+
}
206223
});
207224
connection.on('reconnecting', (): void => {
208-
this.logger.log('Reconnecting to the redis database');
225+
lastError = null;
226+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
209227
});
210228
} catch (e) {
211229
reject(e);
212230
}
213231
}) as Redis;
214232
} catch (e) {
233+
connection?.disconnect?.();
215234
tnl?.close?.();
216235
throw e;
217236
}
@@ -228,36 +247,66 @@ export class RedisConnectionFactory {
228247
database: Database,
229248
options: IRedisConnectionOptions,
230249
): Promise<Cluster> {
231-
const config = await this.getRedisClusterOptions(clientMetadata, database, options);
250+
let connection: Cluster;
232251

233-
if (database.ssh) {
234-
throw new Error('SSH is unsupported for cluster databases.');
235-
}
252+
try {
253+
const config = await this.getRedisClusterOptions(clientMetadata, database, options);
236254

237-
return new Promise((resolve, reject) => {
238-
try {
239-
const cluster = new Cluster([{
240-
host: database.host,
241-
port: database.port,
242-
}].concat(database.nodes), {
243-
...config,
244-
});
245-
cluster.on('error', (e): void => {
246-
this.logger.error('Failed connection to the redis oss cluster', e);
247-
reject(!isEmpty(e.lastNodeError) ? e.lastNodeError : e);
248-
});
249-
cluster.on('end', (): void => {
250-
this.logger.error(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION);
251-
reject(new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
252-
});
253-
cluster.on('ready', (): void => {
254-
this.logger.log('Successfully connected to the redis oss cluster.');
255-
resolve(cluster);
256-
});
257-
} catch (e) {
258-
reject(e);
255+
if (database.ssh) {
256+
throw new Error('SSH is unsupported for cluster databases.');
259257
}
260-
});
258+
259+
return await (new Promise((resolve, reject) => {
260+
try {
261+
let lastError: Error;
262+
263+
connection = new Cluster([{
264+
host: database.host,
265+
port: database.port,
266+
}].concat(database.nodes), {
267+
...config,
268+
redisOptions: {
269+
...config.redisOptions,
270+
db: 0,
271+
},
272+
});
273+
connection.on('error', (e): void => {
274+
this.logger.error('Failed connection to the redis oss cluster', e);
275+
lastError = !isEmpty(e.lastNodeError) ? e.lastNodeError : e;
276+
});
277+
connection.on('end', (): void => {
278+
this.logger.error(ERROR_MESSAGES.UNABLE_TO_ESTABLISH_CONNECTION, lastError);
279+
reject(lastError || new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
280+
});
281+
connection.on('ready', (): void => {
282+
lastError = null;
283+
this.logger.log('Successfully connected to the redis oss cluster.');
284+
285+
// manual switch to particular logical db
286+
// since ioredis doesn't handle "select" command error during connection
287+
if (config.redisOptions.db > 0) {
288+
connection.select(config.redisOptions.db)
289+
.then(() => {
290+
set(connection, ['options', 'db'], config.redisOptions.db);
291+
resolve(connection);
292+
})
293+
.catch(reject);
294+
} else {
295+
resolve(connection);
296+
}
297+
});
298+
connection.on('reconnecting', (): void => {
299+
lastError = null;
300+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
301+
});
302+
} catch (e) {
303+
reject(e);
304+
}
305+
}));
306+
} catch (e) {
307+
connection?.disconnect?.();
308+
throw e;
309+
}
261310
}
262311

263312
/**
@@ -271,27 +320,56 @@ export class RedisConnectionFactory {
271320
database: Database,
272321
options: IRedisConnectionOptions,
273322
): Promise<Redis> {
274-
const config = await this.getRedisSentinelOptions(clientMetadata, database, options);
323+
let connection: Redis;
275324

276-
return new Promise((resolve, reject) => {
277-
try {
278-
const client = new Redis(config);
279-
client.on('error', (e): void => {
280-
this.logger.error('Failed connection to the redis oss sentinel', e);
325+
try {
326+
const config = await this.getRedisSentinelOptions(clientMetadata, database, options);
327+
328+
return await (new Promise((resolve, reject) => {
329+
try {
330+
let lastError: Error;
331+
332+
connection = new Redis({
333+
...config,
334+
db: 0,
335+
});
336+
connection.on('error', (e): void => {
337+
this.logger.error('Failed connection to the redis oss sentinel', e);
338+
lastError = e;
339+
});
340+
connection.on('end', (): void => {
341+
this.logger.error(ERROR_MESSAGES.UNABLE_TO_ESTABLISH_CONNECTION, lastError);
342+
reject(lastError || new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
343+
});
344+
connection.on('ready', (): void => {
345+
lastError = null;
346+
this.logger.log('Successfully connected to the redis oss sentinel.');
347+
348+
// manual switch to particular logical db
349+
// since ioredis doesn't handle "select" command error during connection
350+
if (config.db > 0) {
351+
connection.select(config.db)
352+
.then(() => {
353+
set(connection, ['options', 'db'], config.db);
354+
resolve(connection);
355+
})
356+
.catch(reject);
357+
} else {
358+
resolve(connection);
359+
}
360+
});
361+
connection.on('reconnecting', (): void => {
362+
lastError = null;
363+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
364+
});
365+
} catch (e) {
281366
reject(e);
282-
});
283-
client.on('end', (): void => {
284-
this.logger.error(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION);
285-
reject(new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
286-
});
287-
client.on('ready', (): void => {
288-
this.logger.log('Successfully connected to the redis oss sentinel.');
289-
resolve(client);
290-
});
291-
} catch (e) {
292-
reject(e);
293-
}
294-
});
367+
}
368+
}));
369+
} catch (e) {
370+
connection?.disconnect?.();
371+
throw e;
372+
}
295373
}
296374

297375
/**

0 commit comments

Comments
 (0)