Skip to content

Commit c78e5b6

Browse files
Merge pull request #1848 from RedisInsight/main
merge main
2 parents fe7807f + b04c050 commit c78e5b6

File tree

25 files changed

+956
-350
lines changed

25 files changed

+956
-350
lines changed

redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,5 +303,66 @@ describe('RedisearchService', () => {
303303
expect(e).toBeInstanceOf(ForbiddenException);
304304
}
305305
});
306+
it('should call once "FT.CONFIG GET MAXSEARCHRESULTS" for all requests', async () => {
307+
when(nodeClient.sendCommand)
308+
.calledWith(jasmine.objectContaining({ name: 'FT.SEARCH' }))
309+
.mockResolvedValue([100, keyName1, keyName2]);
310+
when(nodeClient.sendCommand)
311+
.calledWith(jasmine.objectContaining({ name: 'FT.CONFIG' }))
312+
.mockResolvedValue([['MAXSEARCHRESULTS', '10000']]);
313+
314+
const res = await service.search(mockBrowserClientMetadata, mockSearchRedisearchDto);
315+
await service.search(mockBrowserClientMetadata, mockSearchRedisearchDto);
316+
await service.search(mockBrowserClientMetadata, mockSearchRedisearchDto);
317+
318+
expect(res).toEqual({
319+
cursor: mockSearchRedisearchDto.limit + mockSearchRedisearchDto.offset,
320+
scanned: 2,
321+
total: 100,
322+
maxResults: 10_000,
323+
keys: [{
324+
name: keyName1,
325+
}, {
326+
name: keyName2,
327+
}],
328+
});
329+
330+
expect(nodeClient.sendCommand).toHaveBeenCalledTimes(4);
331+
expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
332+
name: 'FT.SEARCH',
333+
args: [
334+
mockSearchRedisearchDto.index,
335+
mockSearchRedisearchDto.query,
336+
'NOCONTENT',
337+
'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
338+
],
339+
}));
340+
expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
341+
name: 'FT.CONFIG',
342+
args: [
343+
'GET',
344+
'MAXSEARCHRESULTS',
345+
],
346+
}));
347+
expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
348+
name: 'FT.SEARCH',
349+
args: [
350+
mockSearchRedisearchDto.index,
351+
mockSearchRedisearchDto.query,
352+
'NOCONTENT',
353+
'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
354+
],
355+
}));
356+
expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
357+
name: 'FT.SEARCH',
358+
args: [
359+
mockSearchRedisearchDto.index,
360+
mockSearchRedisearchDto.query,
361+
'NOCONTENT',
362+
'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
363+
],
364+
}));
365+
expect(browserHistory.create).toHaveBeenCalled();
366+
});
306367
});
307368
});

redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Cluster, Command, Redis } from 'ioredis';
2-
import { toNumber, uniq } from 'lodash';
2+
import { isNull, toNumber, uniq } from 'lodash';
33
import {
44
BadRequestException,
55
ConflictException,
@@ -26,6 +26,8 @@ import { CreateBrowserHistoryDto } from '../../dto/browser-history/create.browse
2626

2727
@Injectable()
2828
export class RedisearchService {
29+
private maxSearchResults: number | null = null;
30+
2931
private logger = new Logger('RedisearchService');
3032

3133
constructor(
@@ -145,30 +147,30 @@ export class RedisearchService {
145147
this.logger.log('Searching keys using redisearch.');
146148

147149
try {
148-
let maxResults;
149150
const {
150151
index, query, offset, limit,
151152
} = dto;
152153

153154
const client = await this.browserTool.getRedisClient(clientMetadata);
154155

155-
try {
156-
const [[, maxSearchResults]] = await client.sendCommand(
157-
// response: [ [ 'MAXSEARCHRESULTS', '10000' ] ]
158-
new Command('FT.CONFIG', ['GET', 'MAXSEARCHRESULTS'], {
159-
replyEncoding: 'utf8',
160-
}),
161-
) as [[string, string]];
162-
163-
maxResults = toNumber(maxSearchResults);
164-
} catch (error) {
165-
maxResults = null;
156+
if (isNull(this.maxSearchResults)) {
157+
try {
158+
const [[, maxSearchResults]] = await client.sendCommand(
159+
// response: [ [ 'MAXSEARCHRESULTS', '10000' ] ]
160+
new Command('FT.CONFIG', ['GET', 'MAXSEARCHRESULTS'], {
161+
replyEncoding: 'utf8',
162+
}),
163+
) as [[string, string]];
164+
165+
this.maxSearchResults = toNumber(maxSearchResults);
166+
} catch (error) {
167+
this.maxSearchResults = null;
168+
}
166169
}
167-
168170
// Workaround: recalculate limit to not query more then MAXSEARCHRESULTS
169171
let safeLimit = limit;
170-
if (maxResults && offset + limit > maxResults) {
171-
safeLimit = offset <= maxResults ? maxResults - offset : limit;
172+
if (this.maxSearchResults && offset + limit > this.maxSearchResults) {
173+
safeLimit = offset <= this.maxSearchResults ? this.maxSearchResults - offset : limit;
172174
}
173175

174176
const [total, ...keyNames] = await client.sendCommand(
@@ -191,7 +193,7 @@ export class RedisearchService {
191193
total,
192194
scanned: keyNames.length + offset,
193195
keys: keyNames.map((name) => ({ name })),
194-
maxResults,
196+
maxResults: this.maxSearchResults,
195197
});
196198
} catch (e) {
197199
this.logger.error('Failed to search keys using redisearch index', e);

redisinsight/api/src/modules/database/providers/database-overview.provider.spec.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ const mockNodeInfo = {
5757
keyspace: mockKeyspace,
5858
};
5959

60-
const mockGetTotalResponse_1 = 1;
60+
const mockGetTotalResponse1 = 1;
6161

6262
export const mockDatabaseOverview: DatabaseOverview = {
6363
version: mockServerInfo.redis_version,
@@ -127,6 +127,42 @@ describe('OverviewService', () => {
127127
totalKeysPerDb: undefined,
128128
});
129129
});
130+
it('should return total 3 and empty total per db object (even when role is not master)', async () => {
131+
spyGetNodeInfo.mockResolvedValueOnce({
132+
...mockNodeInfo,
133+
replication: {
134+
role: 'slave',
135+
},
136+
keyspace: {
137+
db0: 'keys=3,expires=0,avg_ttl=0',
138+
},
139+
});
140+
141+
expect(await service.getOverview(mockClientMetadata, mockClient)).toEqual({
142+
...mockDatabaseOverview,
143+
totalKeys: 3,
144+
totalKeysPerDb: undefined,
145+
});
146+
});
147+
it('should not return particular fields when metrics are not available', async () => {
148+
spyGetNodeInfo.mockResolvedValueOnce({
149+
...mockNodeInfo,
150+
replication: {
151+
role: 'slave',
152+
},
153+
keyspace: undefined,
154+
memory: undefined,
155+
clients: undefined,
156+
});
157+
158+
expect(await service.getOverview(mockClientMetadata, mockClient)).toEqual({
159+
...mockDatabaseOverview,
160+
totalKeys: undefined,
161+
usedMemory: undefined,
162+
totalKeysPerDb: undefined,
163+
connectedClients: undefined,
164+
});
165+
});
130166
it('check for cpu on second attempt', async () => {
131167
spyGetNodeInfo.mockResolvedValueOnce(mockNodeInfo);
132168
spyGetNodeInfo.mockResolvedValueOnce({
@@ -197,7 +233,7 @@ describe('OverviewService', () => {
197233
});
198234
describe('Cluster', () => {
199235
it('Should calculate overview and ignore replica where needed', async () => {
200-
const getTotal = jest.spyOn(Utils, 'getTotal').mockResolvedValue(mockGetTotalResponse_1);
236+
const getTotal = jest.spyOn(Utils, 'getTotal').mockResolvedValue(mockGetTotalResponse1);
201237
mockCluster.nodes = jest.fn()
202238
.mockReturnValue(new Array(6).fill(Promise.resolve()));
203239

redisinsight/api/src/modules/database/providers/database-overview.provider.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,7 @@ export class DatabaseOverviewProvider {
187187
*/
188188
private calculateUsedMemory(nodes = []): number {
189189
try {
190-
const masterNodes = filter(nodes, (node) => ['master', undefined].includes(
191-
get(node, 'replication.role'),
192-
));
190+
const masterNodes = DatabaseOverviewProvider.getMasterNodesToWorkWith(nodes);
193191

194192
if (!this.isMetricsAvailable(masterNodes, 'memory.used_memory', [undefined])) {
195193
return undefined;
@@ -209,14 +207,12 @@ export class DatabaseOverviewProvider {
209207
* @private
210208
*/
211209
private calculateTotalKeys(nodes = [], index: number): [number, Record<string, number>] {
212-
if (!this.isMetricsAvailable(nodes, 'keyspace', [undefined])) {
213-
return [undefined, undefined];
214-
}
215-
216210
try {
217-
const masterNodes = filter(nodes, (node) => ['master', undefined].includes(
218-
get(node, 'replication.role'),
219-
));
211+
const masterNodes = DatabaseOverviewProvider.getMasterNodesToWorkWith(nodes);
212+
213+
if (!this.isMetricsAvailable(masterNodes, 'keyspace', [undefined])) {
214+
return [undefined, undefined];
215+
}
220216

221217
const totalKeysPerDb = {};
222218

@@ -334,4 +330,16 @@ export class DatabaseOverviewProvider {
334330

335331
return true;
336332
}
333+
334+
static getMasterNodesToWorkWith(nodes = []): any[] {
335+
let masterNodes = nodes;
336+
337+
if (nodes?.length > 1) {
338+
masterNodes = filter(nodes, (node) => ['master', undefined].includes(
339+
get(node, 'replication.role'),
340+
));
341+
}
342+
343+
return masterNodes;
344+
}
337345
}
Lines changed: 8 additions & 0 deletions
Loading
Lines changed: 10 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)