chore: removed ping in favor of ioredis keepalive (#86)

We observed that `limitd-redis` fails to promptly detect a stale TCP connection to Redis, only realizing the connection is down after approximately 12 minutes. After this delay, it reconnects as expected.

This update enables the `keepAlive` feature in ioredis, which sets the `KEEPALIVE` option on the socket. This allows for a quicker reaction to stale connections.

Key changes:
- The `keepAlive` feature is enabled by default and set to 10000 ms.
- The `keepAlive` interval can be customized by passing the `keepAlive` argument to the `LimitdRedis` constructor.
- Setting the `keepAlive` argument to a non-numeric value will disable the feature.
- The manually-implemented ping mechanism against Redis has been removed, as we now rely on ioredis' `keepAlive` feature.

.github/workflows/ci.yml

@@ -10,6 +10,8 @@ on:
 
 jobs:
   test:
+    env:
+      CI: true
     timeout-minutes: 10
     runs-on: ubuntu-latest
     strategy:

README.md

@@ -9,7 +9,6 @@ It's a fork from [LimitDB](https://github.com/limitd/limitdb).
 - [Configure](#configure)
    - [Options available](#options-available)
    - [Buckets](#buckets)
-   - [Ping](#ping)
 - [Overrides](#overrides)
 - [ERL (Elevated Rate Limits)](#erl-elevated-rate-limits)
    - [Prerequisites](#prerequisites)
@@ -55,11 +54,6 @@ const limitd = new Limitd({
     }
   },
   prefix: 'test:',
-  ping: {
-    interval: 1000,
-    maxFailedAttempts: 5,
-    reconnectIfFailed: true
-  },
   username: 'username',
   password: 'password'
 });
@@ -71,9 +65,9 @@ const limitd = new Limitd({
 - `nodes` (array): [Redis Cluster Configuration](https://github.com/luin/ioredis#cluster).
 - `buckets` (object): Setup your bucket types.
 - `prefix` (string): Prefix keys in Redis.
-- `ping` (object): Configure ping to Redis DB.
 - `username` (string): Redis username. This is ignored if not in Cluster mode. Needs Redis >= v6.
 - `password` (string): Redis password.
+- `keepAlive` (number): TCP KeepAlive on the socket expressed in milliseconds. Set to a non-number value to disable keepAlive
 
 ### Buckets:
 
@@ -91,14 +85,6 @@ If you omit `size`, limitdb assumes that `size` is the value of `per_interval`.
 
 If you don't specify a filling rate with `per_interval` or any other `per_x`, the bucket is fixed and you have to manually reset it using `PUT`.
 
-### Ping:
-
-- `interval` (number): represents the time between two consecutive pings. Default: 3000.
-- `maxFailedAttempts` (number): is the allowed number of failed pings before declaring the connection as dead. Default: 5.
-- `reconnectIfFailed` (boolean): indicates whether we should try to reconnect is the connection is declared dead. Default: true.
-
-
-
 ## Overrides
 You can also define `overrides` inside your type definitions as follows:
 

lib/client.js

@@ -31,7 +31,7 @@ class LimitdRedis extends EventEmitter {
 
     this.db = new LimitDBRedis(_.pick(params, [
       'uri', 'nodes', 'buckets', 'prefix', 'slotsRefreshTimeout', 'slotsRefreshInterval',
-      'username', 'password', 'tls', 'dnsLookup', 'globalTTL', 'cacheSize', 'ping']));
+      'username', 'password', 'tls', 'dnsLookup', 'globalTTL', 'cacheSize', 'keepAlive']));
 
     this.db.on('error', (err) => {
       this.emit('error', err);

lib/db.js

@@ -6,35 +6,17 @@ const LRU = require('lru-cache');
 const utils = require('./utils');
 const Redis = require('ioredis');
 const { validateParams, validateERLParams } = require('./validation');
-const DBPing = require("./db_ping");
 const { calculateQuotaExpiration, resolveElevatedParams, isFixedWindowEnabled, removeHashtag } = require('./utils');
 const EventEmitter = require('events').EventEmitter;
 
 const TAKE_LUA = fs.readFileSync(`${__dirname}/take.lua`, "utf8");
 const TAKE_ELEVATED_LUA = fs.readFileSync(`${__dirname}/take_elevated.lua`, "utf8");
 const PUT_LUA = fs.readFileSync(`${__dirname}/put.lua`, "utf8");
 
-const PING_SUCCESS = "successful";
-const PING_ERROR = "error";
-const PING_RECONNECT = "reconnect";
-const PING_RECONNECT_DRY_RUN = "reconnect-dry-run";
-
 const DEFAULT_COMMAND_TIMEOUT = 125; // Milliseconds
+const DEFAULT_KEEPALIVE = 10000; // Milliseconds
 
 class LimitDBRedis extends EventEmitter {
-  static get PING_SUCCESS() {
-    return PING_SUCCESS;
-  }
-  static get PING_ERROR() {
-    return PING_ERROR;
-  }
-  static get PING_RECONNECT() {
-    return PING_RECONNECT;
-  }
-  static get PING_RECONNECT_DRY_RUN() {
-    return PING_RECONNECT_DRY_RUN;
-  }
-
   /**
    * Creates an instance of LimitDB client for Redis.
    * @param {params} params - The configuration for the database and client.
@@ -63,6 +45,7 @@ class LimitDBRedis extends EventEmitter {
       keyPrefix: config.prefix,
       password: config.password,
       tls: config.tls,
+      keepAlive: config.keepAlive || DEFAULT_KEEPALIVE,
       reconnectOnError: (err) => {
         // will force a reconnect when error starts with `READONLY`
         // this code is only triggered when auto-failover is disabled
@@ -88,7 +71,6 @@ class LimitDBRedis extends EventEmitter {
       this.redis = new Redis.Cluster(config.nodes, clusterOptions);
     } else {
       this.redis = new Redis(config.uri, redisOptions);
-      this.#setupPing(config);
     }
 
     this.redis.defineCommand('take', {
@@ -120,24 +102,7 @@ class LimitDBRedis extends EventEmitter {
 
   }
 
-  #setupPing(config) {
-    this.redis.on("ready", () => this.#startPing(config));
-    this.redis.on("close", () => this.#stopPing());
-  }
-
-  #startPing(config) {
-    this.#stopPing();
-    this.ping = new DBPing(config.ping, this.redis);
-    this.ping.on("ping", (data) => this.emit("ping", data));
-  }
-
-  #stopPing() {
-    this.ping?.stop();
-    this.ping?.removeAllListeners();
-  }
-
   close(callback) {
-    this.#stopPing();
     this.redis.quit(callback);
   }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "limitd-redis",
-  "version": "8.3.1",
+  "version": "8.4.0",
   "description": "A database client for limits on top of redis",
   "main": "index.js",
   "repository": {

test/client.tests.js

@@ -35,6 +35,23 @@ module.exports = (clientCreator) => {
         client = clientCreator({ retry: { retries: 5 } });
         assert.equal(client.retryOpts.retries, 5);
       });
+
+      describe('keepAlive', () => {
+        describe('when keepAlive is not set', () => {
+          it('should set the keepAlive config to the default value', () => {
+            client = clientCreator();
+            assert.equal(client.db.redis.options.keepAlive || client.db.redis.options.redisOptions.keepAlive, 10000);
+          });
+        });
+
+        describe('when keepAlive is set', () => {
+          it('should set the keepAlive config to the specified value', () => {
+            client = clientCreator({ keepAlive: 5000 });
+            assert.equal(client.db.redis.options.keepAlive || client.db.redis.options.redisOptions.keepAlive, 5000);
+          });
+        });
+      });
+
     });
 
     describe('#handler', () => {
@@ -196,4 +213,4 @@ module.exports = (clientCreator) => {
       });
     });
   });
-};
+};

test/db.clustermode.tests.js

@@ -28,4 +28,4 @@ describe('when using LimitDB', () => {
       });
     });
   })
-});
+});