diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16da035..c2e71b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,8 @@ on: jobs: test: + env: + CI: true timeout-minutes: 10 runs-on: ubuntu-latest strategy: diff --git a/README.md b/README.md index dc5e939..cc8e47d 100644 --- a/README.md +++ b/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: diff --git a/lib/client.js b/lib/client.js index 0fe058e..574b57a 100644 --- a/lib/client.js +++ b/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); diff --git a/lib/db.js b/lib/db.js index e594798..fdcd689 100644 --- a/lib/db.js +++ b/lib/db.js @@ -6,7 +6,6 @@ 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; @@ -14,27 +13,10 @@ 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); } diff --git a/lib/db_ping.js b/lib/db_ping.js deleted file mode 100644 index f564d61..0000000 --- a/lib/db_ping.js +++ /dev/null @@ -1,106 +0,0 @@ -const cbControl = require('./cb'); -const utils = require("./utils"); -const EventEmitter = require("events").EventEmitter; - -const PING_SUCCESS = "successful"; -const PING_ERROR = "error"; -const PING_RECONNECT = "reconnect"; -const PING_RECONNECT_DRY_RUN = "reconnect-dry-run"; - -const DEFAULT_PING_INTERVAL = 3000; // Milliseconds - -class DBPing extends EventEmitter { - constructor(config, redis) { - super(); - - this.redis = redis; - this.config = { - commandTimeout: 125, - enabled: config ? true : false, - interval: config?.interval || DEFAULT_PING_INTERVAL, - maxFailedAttempts: config?.maxFailedAttempts || 5, - reconnectIfFailed: - utils.functionOrFalse(config?.reconnectIfFailed) || (() => false), - }; - - this.failedPings = 0; - - this.#start(); - } - - #start() { - const doPing = () => { - if (!this.config.enabled) { - return; - } - - let start = Date.now(); - this.redis.ping(cbControl((err) => { - let duration = Date.now() - start; - err - ? this.#pingKO(triggerLoop, err, duration) - : this.#pingOK(triggerLoop, duration); - }).timeout(this.config.commandTimeout)); - }; - - const triggerLoop = () => setTimeout(doPing, this.config.interval); - - doPing(); - } - - stop() { - this.enabled = false; - } - - #pingOK(callback, duration) { - this.reconnecting = false; - this.failedPings = 0; - this.#emitPingResult(PING_SUCCESS, undefined, duration, 0); - callback(); - } - - #pingKO(callback, err, duration) { - this.failedPings++; - this.#emitPingResult(PING_ERROR, err, duration, this.failedPings); - - if (this.failedPings < this.config.maxFailedAttempts) { - return callback(); - } - - if (!this.config.reconnectIfFailed()) { - return this.#emitPingResult( - PING_RECONNECT_DRY_RUN, - undefined, - 0, - this.failedPings - ); - } - - this.#retryStrategy(() => { - this.#emitPingResult(PING_RECONNECT, undefined, 0, this.failedPings); - this.redis.disconnect(true); - }); - } - - #emitPingResult(status, err, duration, failedPings) { - const result = { - status: status, - duration: duration, - error: err, - failedPings: failedPings, - }; - this.emit("ping", result); - } - - #retryStrategy(callback) { - //jitter between 0% and 10% of the total wait time needed to reconnect - //i.e. if interval = 100 and maxFailedAttempts = 3 => it'll randomly jitter between 0 and 30 ms - const deviation = - utils.randomBetween(0, 0.1) * - this.config.interval * - this.config.maxFailedAttempts; - setTimeout(callback, deviation); - } -} - -module.exports = DBPing; diff --git a/package.json b/package.json index 8ede626..a0e3f6f 100644 --- a/package.json +++ b/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": { diff --git a/test/client.tests.js b/test/client.tests.js index f764590..6806820 100644 --- a/test/client.tests.js +++ b/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) => { }); }); }); -}; \ No newline at end of file +}; diff --git a/test/db.clustermode.tests.js b/test/db.clustermode.tests.js index 1dd614a..34c7a89 100644 --- a/test/db.clustermode.tests.js +++ b/test/db.clustermode.tests.js @@ -28,4 +28,4 @@ describe('when using LimitDB', () => { }); }); }) -}); \ No newline at end of file +}); diff --git a/test/db.standalonemode.tests.js b/test/db.standalonemode.tests.js index fb8784e..ce032f0 100644 --- a/test/db.standalonemode.tests.js +++ b/test/db.standalonemode.tests.js @@ -10,7 +10,7 @@ const crypto = require('crypto'); describe('when using LimitDB', () => { describe('in standalone mode', () => { const clientCreator = (params) => { - return new LimitDB({ uri: 'localhost', buckets: {}, prefix: 'tests:', ..._.omit(params, ['nodes']) }); + return new LimitDB({ uri: 'localhost:6379', buckets: {}, prefix: 'tests:', ..._.omit(params, ['nodes']) }); }; dbTests(clientCreator); @@ -27,171 +27,5 @@ describe('when using LimitDB', () => { }); }); }); - - describe('LimitDBRedis Ping', () => { - - let ping = { - enabled: () => true, - interval: 10, - maxFailedAttempts: 3, - reconnectIfFailed: () => true, - maxFailedAttemptsToRetryReconnect: 10 - }; - - let config = { - uri: 'localhost:22222', - buckets, - prefix: 'tests:', - ping, - }; - - let redisProxy; - let toxiproxy; - let db; - - beforeEach((done) => { - toxiproxy = new Toxiproxy('http://localhost:8474'); - proxyBody = { - listen: '0.0.0.0:22222', - name: crypto.randomUUID(), //randomize name to avoid concurrency issues - upstream: 'redis:6379' - }; - toxiproxy.createProxy(proxyBody) - .then((proxy) => { - redisProxy = proxy; - done(); - }); - - }); - - afterEach((done) => { - redisProxy.remove().then(() => - db.close((err) => { - // Can't close DB if it was never open - if (err?.message.indexOf('enableOfflineQueue') > 0 || err?.message.indexOf('Connection is closed') >= 0) { - err = undefined; - } - done(err); - }) - ); - }); - - it('should emit ping success', (done) => { - db = createDB({ uri: 'localhost:22222', buckets, prefix: 'tests:', ping }, done); - db.once(('ping'), (result) => { - if (result.status === LimitDB.PING_SUCCESS) { - done(); - } - }); - }); - - it('should emit "ping - error" when redis stops responding pings', (done) => { - let called = false; - - db = createDB(config, done); - db.once(('ready'), () => addLatencyToxic(redisProxy, 20000, noop)); - db.on(('ping'), (result) => { - if (result.status === LimitDB.PING_ERROR && !called) { - called = true; - db.removeAllListeners('ping'); - done(); - } - }); - }); - - it('should emit "ping - reconnect" when redis stops responding pings and client is configured to reconnect', (done) => { - let called = false; - db = createDB(config, done); - db.once(('ready'), () => addLatencyToxic(redisProxy, 20000, noop)); - db.on(('ping'), (result) => { - if (result.status === LimitDB.PING_RECONNECT && !called) { - called = true; - db.removeAllListeners('ping'); - done(); - } - }); - }); - - it('should emit "ping - reconnect dry run" when redis stops responding pings and client is NOT configured to reconnect', (done) => { - let called = false; - db = createDB({ ...config, ping: { ...ping, reconnectIfFailed: () => false } }, done); - db.once(('ready'), () => addLatencyToxic(redisProxy, 20000, noop)); - db.on(('ping'), (result) => { - if (result.status === LimitDB.PING_RECONNECT_DRY_RUN && !called) { - called = true; - db.removeAllListeners('ping'); - done(); - } - }); - }); - - it(`should NOT emit ping events when config.ping is not set`, (done) => { - db = createDB({ ...config, ping: undefined }, done); - - db.once(('ping'), (result) => { - done(new Error(`unexpected ping event emitted ${result}`)); - }); - - //If after 100ms there are no interactions, we mark the test as passed. - setTimeout(done, 100); - }); - - it('should recover from a connection loss', (done) => { - let pingResponded = false; - let reconnected = false; - let toxic = undefined; - let timeoutId; - db = createDB({ ...config, ping: { ...ping, interval: 50 } }, done); - - db.on(('ping'), (result) => { - if (result.status === LimitDB.PING_SUCCESS) { - if (!pingResponded) { - pingResponded = true; - toxic = addLatencyToxic(redisProxy, 20000, (t) => toxic = t); - } else if (reconnected) { - clearTimeout(timeoutId); - db.removeAllListeners('ping'); - done(); - } - } else if (result.status === LimitDB.PING_RECONNECT) { - if (pingResponded && !reconnected) { - reconnected = true; - toxic.remove(); - } - } - }); - - timeoutId = setTimeout(() => done(new Error('Not reconnected')), 1800); - }); - - const createDB = (config, done) => { - let tmpDB = new LimitDB(config); - - tmpDB.on(('error'), (err) => { - //As we actively close the connection, there might be network-related errors while attempting to reconnect - if (err?.message.indexOf('enableOfflineQueue') > 0 || err?.message.indexOf('Command timed out') >= 0) { - err = undefined; - } - - if (err) { - done(err); - } - }); - - return tmpDB; - }; - - const addLatencyToxic = (proxy, latency, callback) => { - let toxic = new Toxic( - proxy, - { type: 'latency', attributes: { latency: latency } } - ); - proxy.addToxic(toxic).then(callback); - }; - - - const noop = () => { - }; - }); }) -}); \ No newline at end of file +});