Node: Remove timeout on requests.

These timeouts carry a significant performance penalty. Their benefit is more precise timeouts to the user, but we're willing to be off by a bit in order to improve perf.
Bit-Quill · Nov 23, 2023 · 1836f4e · 1836f4e
1 parent f6ac486
commit 1836f4e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 50 deletions.
diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts
@@ -52,7 +52,6 @@ import {
     ExecAbortError,
     RedisError,
     RequestError,
-    TIMEOUT_ERROR,
     TimeoutError,
 } from "./Errors";
 import { Logger } from "./Logger";
@@ -257,9 +256,6 @@ export class BaseClient {
         route?: redis_request.Routes
     ): Promise<T> {
         return new Promise((resolve, reject) => {
-            setTimeout(() => {
-                reject(TIMEOUT_ERROR);
-            }, this.requestTimeout);
             const callbackIndex = this.getCallbackIndex();
             this.promiseCallbackFunctions[callbackIndex] = [resolve, reject];
             this.writeOrBufferRedisRequest(callbackIndex, command, route);

diff --git a/node/tests/RedisClientInternals.test.ts b/node/tests/RedisClientInternals.test.ts
@@ -14,15 +14,18 @@ import {
     RedisClusterClient,
     RequestError,
     TimeoutError,
-    Transaction
+    Transaction,
 } from "../build-ts";
 import { RedisClientConfiguration } from "../build-ts/src/RedisClient";
 import {
     connection_request,
     redis_request,
     response,
 } from "../src/ProtobufMessage";
-import { ClusterClientConfiguration, SlotKeyTypes } from "../src/RedisClusterClient";
+import {
+    ClusterClientConfiguration,
+    SlotKeyTypes,
+} from "../src/RedisClusterClient";
 
 const { RequestType, RedisRequest } = redis_request;
 
@@ -254,24 +257,25 @@ describe("SocketConnectionInternals", () => {
             socket.once("data", (data) => {
                 const reader = Reader.create(data);
                 const request = RedisRequest.decodeDelimited(reader);
-
-                expect(request.transaction?.commands?.at(0)?.requestType).toEqual(
-                    RequestType.SetString
-                );
-                expect(request.transaction?.commands?.at(0)?.argsArray?.args?.length).toEqual(
-                    2
-                );
+
+                expect(
+                    request.transaction?.commands?.at(0)?.requestType
+                ).toEqual(RequestType.SetString);
+                expect(
+                    request.transaction?.commands?.at(0)?.argsArray?.args
+                        ?.length
+                ).toEqual(2);
                 expect(request.route?.slotKeyRoute?.slotKey).toEqual("key");
                 expect(request.route?.slotKeyRoute?.slotType).toEqual(0); // Primary = 0
 
                 sendResponse(socket, ResponseType.OK, request.callbackIdx);
             });
             const transaction = new Transaction();
-            transaction.set("key" , "value");
+            transaction.set("key", "value");
             const slotKey: SlotKeyTypes = {
                 type: "primarySlotKey",
-                key: "key"
-              };
+                key: "key",
+            };
             const result = await connection.exec(transaction, slotKey);
             expect(result).toBe("OK");
         });
@@ -282,16 +286,24 @@ describe("SocketConnectionInternals", () => {
             socket.once("data", (data) => {
                 const reader = Reader.create(data);
                 const request = RedisRequest.decodeDelimited(reader);
-
-                expect(request.transaction?.commands?.at(0)?.requestType).toEqual(
-                    RequestType.Info
-                );
-                expect(request.transaction?.commands?.at(0)?.argsArray?.args?.length).toEqual(
-                    1
+
+                expect(
+                    request.transaction?.commands?.at(0)?.requestType
+                ).toEqual(RequestType.Info);
+                expect(
+                    request.transaction?.commands?.at(0)?.argsArray?.args
+                        ?.length
+                ).toEqual(1);
+                expect(request.route?.simpleRoutes).toEqual(
+                    redis_request.SimpleRoutes.Random
                 );
-                expect(request.route?.simpleRoutes).toEqual(redis_request.SimpleRoutes.Random);
 
-                sendResponse(socket, ResponseType.Value, request.callbackIdx , "# Server");
+                sendResponse(
+                    socket,
+                    ResponseType.Value,
+                    request.callbackIdx,
+                    "# Server"
+                );
             });
             const transaction = new Transaction();
             transaction.info([InfoOptions.Server]);
@@ -495,32 +507,6 @@ describe("SocketConnectionInternals", () => {
         closeTestResources(connection, server, socket);
     });
 
-    it("should timeout before receiving response from core", async () => {
-        await testWithResources(
-            async (connection, socket) => {
-                socket.once("data", (data) =>
-                    setTimeout(() => {
-                        const reader = Reader.create(data);
-                        const request = RedisRequest.decodeDelimited(reader);
-                        expect(request.singleCommand?.requestType).toEqual(
-                            RequestType.GetString
-                        );
-                        expect(
-                            request.singleCommand?.argsArray?.args?.length
-                        ).toEqual(1);
-                    }, 20)
-                );
-                await expect(connection.get("foo")).rejects.toThrow(
-                    TimeoutError
-                );
-            },
-            {
-                addresses: [{ host: "foo" }],
-                requestTimeout: 1,
-            }
-        );
-    });
-
     it("should pass routing information from user", async () => {
         const route1 = "allPrimaries";
         const route2 = {