Skip to content

Commit 5c49cbd

Browse files
author
KIvanow
committed
testing delaying of the autodiscovery as a way to avoid the odd race condition happening
1 parent a81ed32 commit 5c49cbd

File tree

4 files changed

+57
-23
lines changed

4 files changed

+57
-23
lines changed

redisinsight/api/src/modules/database-discovery/auto.database-discovery.service.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,30 @@ import { ConstantsProvider } from 'src/modules/constants/providers/constants.pro
1515
@Injectable()
1616
export class AutoDatabaseDiscoveryService {
1717
private logger = new Logger('AutoDatabaseDiscoveryService');
18+
private isDiscoveryRunning = false;
1819

1920
constructor(
2021
private redisClientFactory: RedisClientFactory,
2122
private databaseService: DatabaseService,
2223
) {}
2324

2425
/**
25-
* Try to add standalone databases without auth from processes running on the host machine listening on TCP4
26+
* Start discovery process in background without blocking the main thread
2627
* Database alias will be "host:port"
2728
*/
2829
async discover(sessionMetadata: SessionMetadata) {
30+
// Return immediately to not block the main thread
31+
if (this.isDiscoveryRunning) {
32+
return;
33+
}
34+
35+
// Perform the actual discovery in background
36+
this.isDiscoveryRunning = true;
2937
try {
3038
// additional check for existing databases
3139
// We should not start auto discover if any database already exists
3240
if ((await this.databaseService.list(sessionMetadata)).length) {
41+
this.isDiscoveryRunning = false;
3342
return;
3443
}
3544

@@ -46,6 +55,8 @@ export class AutoDatabaseDiscoveryService {
4655
]);
4756
} catch (e) {
4857
this.logger.warn('Unable to discover redis database', e);
58+
} finally {
59+
this.isDiscoveryRunning = false;
4960
}
5061
}
5162

redisinsight/api/src/modules/database-discovery/local.database-discovery.service.ts

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const SERVER_CONFIG = config.get('server') as Config['server'];
1111
@Injectable()
1212
export class LocalDatabaseDiscoveryService extends DatabaseDiscoveryService {
1313
private logger = new Logger('LocalDatabaseDiscoveryService');
14+
private isDiscoveryRunning = false;
1415

1516
constructor(
1617
@Inject(forwardRef(() => SettingsService))
@@ -21,33 +22,49 @@ export class LocalDatabaseDiscoveryService extends DatabaseDiscoveryService {
2122
super();
2223
}
2324

25+
/**
26+
* Non-blocking implementation of database discovery
27+
* This returns quickly and performs the actual discovery work in the background
28+
*/
2429
async discover(
2530
sessionMetadata: SessionMetadata,
2631
firstRun?: boolean,
2732
): Promise<void> {
28-
try {
29-
// no need to auto discover for Redis Stack
30-
if (SERVER_CONFIG.buildType === 'REDIS_STACK') {
31-
return;
32-
}
33+
// Return immediately if discovery is already in progress
34+
if (this.isDiscoveryRunning) {
35+
return;
36+
}
37+
38+
// No need to auto discover for Redis Stack - quick check
39+
if (SERVER_CONFIG.buildType === 'REDIS_STACK') {
40+
return;
41+
}
3342

34-
// check agreements to understand if it is first launch
35-
const settings =
36-
await this.settingsService.getAppSettings(sessionMetadata);
43+
// Mark as running and perform discovery in background
44+
this.isDiscoveryRunning = true;
45+
setImmediate(async () => {
46+
try {
47+
// check agreements to understand if it is first launch
48+
const settings =
49+
await this.settingsService.getAppSettings(sessionMetadata);
3750

38-
if (!settings?.agreements?.eula) {
39-
return;
40-
}
51+
if (!settings?.agreements?.eula) {
52+
this.isDiscoveryRunning = false;
53+
return;
54+
}
4155

42-
const { discovered } =
43-
await this.preSetupDatabaseDiscoveryService.discover(sessionMetadata);
56+
const { discovered } =
57+
await this.preSetupDatabaseDiscoveryService.discover(sessionMetadata);
4458

45-
if (!discovered && firstRun) {
46-
await this.autoDatabaseDiscoveryService.discover(sessionMetadata);
59+
if (!discovered && firstRun) {
60+
await this.autoDatabaseDiscoveryService.discover(sessionMetadata);
61+
}
62+
} catch (e) {
63+
// ignore error
64+
this.logger.error('Unable to discover databases', e);
65+
} finally {
66+
this.isDiscoveryRunning = false;
4767
}
48-
} catch (e) {
49-
// ignore error
50-
this.logger.error('Unable to discover databases', e);
51-
}
68+
});
5269
}
5370
}

redisinsight/api/src/modules/database-discovery/utils/autodiscovery.util.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,23 @@ export const getRunningProcesses = async (): Promise<string[]> =>
2929
let stdoutData = '';
3030
const proc = spawn(...getSpawnArgs());
3131

32+
// Add timeout to ensure we don't wait forever
33+
const timeout = setTimeout(() => {
34+
proc.kill();
35+
resolve([]); // Return empty array on timeout
36+
}, 1000);
37+
3238
proc.stdout.on('data', (data) => {
3339
stdoutData += data.toString();
3440
});
3541

3642
proc.on('error', (e) => {
43+
clearTimeout(timeout);
3744
reject(e);
3845
});
3946

4047
proc.stdout.on('end', () => {
48+
clearTimeout(timeout);
4149
resolve(stdoutData.split('\n'));
4250
});
4351
} catch (e) {

redisinsight/api/src/modules/init/local.init.service.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ export class LocalInitService extends InitService {
3232
await this.initAnalytics(firstStart);
3333
await this.featureService.recalculateFeatureFlags(sessionMetadata);
3434
await this.redisClientFactory.init();
35-
process.nextTick(async () => {
36-
await this.databaseDiscoveryService.discover(sessionMetadata, firstStart);
37-
});
35+
await this.databaseDiscoveryService.discover(sessionMetadata, firstStart);
3836
}
3937

4038
async initAnalytics(firstStart: boolean) {

0 commit comments

Comments
 (0)