#RI-3844 rework workaround for blocked client due to ft.search command

Author: Artem

redisinsight/api/config/default.ts

@@ -56,7 +56,6 @@ export default {
     buildType: process.env.BUILD_TYPE || 'ELECTRON',
     appVersion: process.env.APP_VERSION || '2.0.0',
     requestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 10000,
-    ftSearchRequestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 45_000,
     excludeRoutes: [],
   },
   sockets: {

redisinsight/api/config/test.ts

@@ -2,7 +2,6 @@ export default {
   server: {
     env: 'test',
     requestTimeout: 1000,
-    ftSearchRequestTimeout: 1000,
   },
   profiler: {
     logFileIdleThreshold: parseInt(process.env.PROFILER_LOG_FILE_IDLE_THRESHOLD, 10) || 1000 * 2, // 3sec

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

@@ -11,7 +11,6 @@ export default {
   WRONG_DATABASE_TYPE: 'Wrong database type.',
   CONNECTION_TIMEOUT:
     'The connection has timed out, please check the connection details.',
-  FT_SEARCH_COMMAND_TIMED_OUT: 'Command timed out.',
   AUTHENTICATION_FAILED: () => 'Failed to authenticate, please check the username or password.',
   INCORRECT_DATABASE_URL: (url) => `Could not connect to ${url}, please check the connection details.`,
   INCORRECT_CERTIFICATES: (url) => `Could not connect to ${url}, please check the CA or Client certificate.`,

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

@@ -2,9 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing';
 import {
   ConflictException,
   ForbiddenException,
-  GatewayTimeoutException,
 } from '@nestjs/common';
-import ERROR_MESSAGES from 'src/constants/error-messages';
 import { when } from 'jest-when';
 import {
   mockDatabase,
@@ -242,7 +240,7 @@ describe('RedisearchService', () => {
           'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
         ],
       }));
-      expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( {
+      expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
         name: 'FT.CONFIG',
         args: [
           'GET',
@@ -279,7 +277,7 @@ describe('RedisearchService', () => {
           'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`,
         ],
       }));
-      expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( {
+      expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({
         name: 'FT.CONFIG',
         args: [
           'GET',
@@ -299,20 +297,5 @@ describe('RedisearchService', () => {
         expect(e).toBeInstanceOf(ForbiddenException);
       }
     });
-    it('should handle produce BadGateway issue due to no response from ft.search command', async () => {
-      when(nodeClient.sendCommand)
-        .calledWith(jasmine.objectContaining({ name: 'FT.SEARCH' }))
-        .mockResolvedValue(new Promise((res) => {
-          setTimeout(() => res([100, keyName1, keyName2]), 1100);
-        }));
-
-      try {
-        await service.search(mockClientOptions, mockSearchRedisearchDto);
-        fail();
-      } catch (e) {
-        expect(e).toBeInstanceOf(GatewayTimeoutException);
-        expect(e.message).toEqual(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT);
-      }
-    });
   });
 });

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

@@ -3,7 +3,6 @@ import { toNumber, uniq } from 'lodash';
 import {
   BadRequestException,
   ConflictException,
-  GatewayTimeoutException,
   HttpException,
   Injectable,
   Logger,
@@ -20,11 +19,8 @@ import { GetKeysWithDetailsResponse } from 'src/modules/browser/dto';
 import { RedisErrorCodes } from 'src/constants';
 import { plainToClass } from 'class-transformer';
 import { numberWithSpaces } from 'src/utils/base.helper';
-import config from 'src/utils/config';
 import { BrowserToolService } from '../browser-tool/browser-tool.service';
 
-const serverConfig = config.get('server');
-
 @Injectable()
 export class RedisearchService {
   private logger = new Logger('RedisearchService');
@@ -152,22 +148,6 @@ export class RedisearchService {
 
       const client = await this.browserTool.getRedisClient(clientOptions);
 
-      // special workaround to avoid blocking client with ft.search command
-      // due to RediSearch issue
-      const [total, ...keyNames] = await Promise.race([
-        client.sendCommand(
-          new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, limit]),
-        ),
-        new Promise((res, rej) => setTimeout(() => {
-          try {
-            client.disconnect();
-          } catch (e) {
-            // ignore any error related to disconnect client
-          }
-          rej(new GatewayTimeoutException(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT));
-        }, serverConfig.ftSearchRequestTimeout)),
-      ]);
-
       try {
         const [[, maxSearchResults]] = await client.sendCommand(
           // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ]
@@ -181,6 +161,16 @@ export class RedisearchService {
         maxResults = null;
       }
 
+      // Workaround: recalculate limit to not query more then MAXSEARCHRESULTS
+      let safeLimit = limit;
+      if (maxResults && offset + limit > maxResults) {
+        safeLimit = offset <= maxResults ? maxResults - offset : limit;
+      }
+
+      const [total, ...keyNames] = await client.sendCommand(
+        new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, safeLimit]),
+      );
+
       return plainToClass(GetKeysWithDetailsResponse, {
         cursor: limit + offset,
         total,

redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts

@@ -45,6 +45,7 @@ const mainCheckFn = getMainCheckFn(endpoint);
 describe('POST /databases/:id/redisearch/search', () => {
   requirements('!rte.bigData', 'rte.modules.search');
   before(async () => rte.data.generateRedisearchIndexes(true));
+  beforeEach(() => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000'));
 
   describe('Main', () => {
     describe('Validation', () => {
@@ -83,17 +84,36 @@ describe('POST /databases/:id/redisearch/search', () => {
             expect(body.maxResults).to.gte(10000);
           },
         },
+        {
+          name: 'Should modify limit to not exceed available search limitation',
+          data: {
+            ...validInputData,
+            offset: 0,
+            limit: 10,
+          },
+          checkFn: async ({ body }) => {
+            expect(body.keys.length).to.eq(1);
+            expect(body.cursor).to.eq(10);
+            expect(body.scanned).to.eq(1);
+            expect(body.total).to.eq(2000);
+            expect(body.maxResults).to.gte(1);
+          },
+          before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'),
+        },
         {
           name: 'Should return custom error message if MAXSEARCHRESULTS less than request.limit',
-          data: validInputData,
+          data: {
+            ...validInputData,
+            offset: 10,
+            limit: 10,
+          },
           statusCode: 400,
           responseBody: {
             statusCode: 400,
             error: 'Bad Request',
             message: `Set MAXSEARCHRESULTS to at least ${numberWithSpaces(validInputData.limit)}.`,
           },
           before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'),
-          after: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000'),
         },
       ].map(mainCheckFn);
     });