Skip to content

Commit 71912ce

Browse files
author
Artem
authored
Merge pull request #2799 from RedisInsight/be/feature/RI-5167-connection-retry
Be/feature/ri 5167 connection retry
2 parents eff999f + 6d8637c commit 71912ce

File tree

5 files changed

+164
-61
lines changed

5 files changed

+164
-61
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: 123 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -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,42 @@ 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(() => resolve(connection))
216+
.catch(reject);
217+
} else {
218+
resolve(connection);
219+
}
206220
});
207221
connection.on('reconnecting', (): void => {
208-
this.logger.log('Reconnecting to the redis database');
222+
lastError = null;
223+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
209224
});
210225
} catch (e) {
211226
reject(e);
212227
}
213228
}) as Redis;
214229
} catch (e) {
230+
connection?.disconnect?.();
215231
tnl?.close?.();
216232
throw e;
217233
}
@@ -228,36 +244,63 @@ export class RedisConnectionFactory {
228244
database: Database,
229245
options: IRedisConnectionOptions,
230246
): Promise<Cluster> {
231-
const config = await this.getRedisClusterOptions(clientMetadata, database, options);
247+
let connection: Cluster;
232248

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

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);
252+
if (database.ssh) {
253+
throw new Error('SSH is unsupported for cluster databases.');
259254
}
260-
});
255+
256+
return await (new Promise((resolve, reject) => {
257+
try {
258+
let lastError: Error;
259+
260+
connection = new Cluster([{
261+
host: database.host,
262+
port: database.port,
263+
}].concat(database.nodes), {
264+
...config,
265+
redisOptions: {
266+
...config.redisOptions,
267+
db: 0,
268+
},
269+
});
270+
connection.on('error', (e): void => {
271+
this.logger.error('Failed connection to the redis oss cluster', e);
272+
lastError = !isEmpty(e.lastNodeError) ? e.lastNodeError : e;
273+
});
274+
connection.on('end', (): void => {
275+
this.logger.error(ERROR_MESSAGES.UNABLE_TO_ESTABLISH_CONNECTION, lastError);
276+
reject(lastError || new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
277+
});
278+
connection.on('ready', (): void => {
279+
lastError = null;
280+
this.logger.log('Successfully connected to the redis oss cluster.');
281+
282+
// manual switch to particular logical db
283+
// since ioredis doesn't handle "select" command error during connection
284+
if (config.redisOptions.db > 0) {
285+
connection.select(config.redisOptions.db)
286+
.then(() => resolve(connection))
287+
.catch(reject);
288+
} else {
289+
resolve(connection);
290+
}
291+
});
292+
connection.on('reconnecting', (): void => {
293+
lastError = null;
294+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
295+
});
296+
} catch (e) {
297+
reject(e);
298+
}
299+
}));
300+
} catch (e) {
301+
connection?.disconnect?.();
302+
throw e;
303+
}
261304
}
262305

263306
/**
@@ -271,27 +314,53 @@ export class RedisConnectionFactory {
271314
database: Database,
272315
options: IRedisConnectionOptions,
273316
): Promise<Redis> {
274-
const config = await this.getRedisSentinelOptions(clientMetadata, database, options);
317+
let connection: Redis;
275318

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);
319+
try {
320+
const config = await this.getRedisSentinelOptions(clientMetadata, database, options);
321+
322+
return await (new Promise((resolve, reject) => {
323+
try {
324+
let lastError: Error;
325+
326+
connection = new Redis({
327+
...config,
328+
db: 0,
329+
});
330+
connection.on('error', (e): void => {
331+
this.logger.error('Failed connection to the redis oss sentinel', e);
332+
lastError = e;
333+
});
334+
connection.on('end', (): void => {
335+
this.logger.error(ERROR_MESSAGES.UNABLE_TO_ESTABLISH_CONNECTION, lastError);
336+
reject(lastError || new InternalServerErrorException(ERROR_MESSAGES.SERVER_CLOSED_CONNECTION));
337+
});
338+
connection.on('ready', (): void => {
339+
lastError = null;
340+
this.logger.log('Successfully connected to the redis oss sentinel.');
341+
342+
// manual switch to particular logical db
343+
// since ioredis doesn't handle "select" command error during connection
344+
if (config.db > 0) {
345+
connection.select(config.db)
346+
.then(() => resolve(connection))
347+
.catch(reject);
348+
} else {
349+
resolve(connection);
350+
}
351+
});
352+
connection.on('reconnecting', (): void => {
353+
lastError = null;
354+
this.logger.log(ERROR_MESSAGES.RECONNECTING_TO_DATABASE);
355+
});
356+
} catch (e) {
281357
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-
});
358+
}
359+
}));
360+
} catch (e) {
361+
connection?.disconnect?.();
362+
throw e;
363+
}
295364
}
296365

297366
/**

0 commit comments

Comments
 (0)