From e076c0e5f53d8696d65287fca328427b52de928a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 15:10:51 +0100 Subject: [PATCH 01/14] Use typed redis clients --- packages/backend-core/src/redis/redis.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index d15453ba62..2280c3f6df 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -28,7 +28,7 @@ const DEFAULT_SELECT_DB = SelectableDatabase.DEFAULT // for testing just generate the client once let CLOSED = false -let CLIENTS: { [key: number]: any } = {} +const CLIENTS: Record = {} let CONNECTED = false // mock redis always connected @@ -36,7 +36,7 @@ if (env.MOCK_REDIS) { CONNECTED = true } -function pickClient(selectDb: number): any { +function pickClient(selectDb: number) { return CLIENTS[selectDb] } From a5d6d094e63def29122ce5d3b73470681523688f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 16:28:31 +0100 Subject: [PATCH 02/14] Update types --- packages/backend-core/package.json | 2 +- packages/types/package.json | 2 +- yarn.lock | 11 ++++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index 3f8c34f823..90b3316c3f 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -67,7 +67,7 @@ "@types/lodash": "4.14.200", "@types/node-fetch": "2.6.4", "@types/pouchdb": "6.4.0", - "@types/redlock": "4.0.3", + "@types/redlock": "4.0.7", "@types/semver": "7.3.7", "@types/tar-fs": "2.0.1", "@types/uuid": "8.3.4", diff --git a/packages/types/package.json b/packages/types/package.json index ce4fce95fb..558e55a632 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -18,7 +18,7 @@ "@budibase/nano": "10.1.5", "@types/koa": "2.13.4", "@types/pouchdb": "6.4.0", - "@types/redlock": "4.0.3", + "@types/redlock": "4.0.7", "rimraf": "3.0.2", "typescript": "5.2.2" }, diff --git a/yarn.lock b/yarn.lock index 260ae3870a..2f9f558e2c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5408,7 +5408,7 @@ resolved "https://registry.yarnpkg.com/@types/http-errors/-/http-errors-2.0.1.tgz#20172f9578b225f6c7da63446f56d4ce108d5a65" integrity sha512-/K3ds8TRAfBvi5vfjuz8y6+GiAYBZ0x4tXv1Av6CWBWn0IlADc+ZX9pMq7oU0fNQPnBwIZl3rmeLp6SBApbxSQ== -"@types/ioredis@4.28.10": +"@types/ioredis@4.28.10", "@types/ioredis@^4.28.10": version "4.28.10" resolved "https://registry.yarnpkg.com/@types/ioredis/-/ioredis-4.28.10.tgz#40ceb157a4141088d1394bb87c98ed09a75a06ff" integrity sha512-69LyhUgrXdgcNDv7ogs1qXZomnfOEnSmrmMFqKgt1XMJxmoOSG/u3wYy13yACIfKuMJ8IhKgHafDO3sx19zVQQ== @@ -5896,12 +5896,13 @@ dependencies: "@types/node" "*" -"@types/redlock@4.0.3": - version "4.0.3" - resolved "https://registry.yarnpkg.com/@types/redlock/-/redlock-4.0.3.tgz#aeab5fe5f0d433a125f6dcf9a884372ac0cddd4b" - integrity sha512-mcvvrquwREbAqyZALNBIlf49AL9Aa324BG+J/Dv4TAP8g+nxQMBI4/APNqqS99QEY7VTNT9XvsaczCVGK8uNnQ== +"@types/redlock@4.0.7": + version "4.0.7" + resolved "https://registry.yarnpkg.com/@types/redlock/-/redlock-4.0.7.tgz#33ed56f22a38d6b2f2e6ae5ed1b3fc1875a08e6b" + integrity sha512-5D6egBv0fCfdbmnCETjEynVuiwFMEFFc3YFjh9EwhaaVTAi0YmB6UI1swq1S1rjIu+n27ppmlTFDK3D3cadJqg== dependencies: "@types/bluebird" "*" + "@types/ioredis" "^4.28.10" "@types/redis" "^2.8.0" "@types/request@^2.48.7": From 56870bed5b0bc2c4479c4107e78193e36195a795 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 16:30:45 +0100 Subject: [PATCH 03/14] Typings --- packages/backend-core/src/redis/redis.ts | 21 ++++++++++++------- .../backend-core/src/redis/redlockImpl.ts | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 2280c3f6df..f8f0c9f3d7 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -1,5 +1,5 @@ import env from "../environment" -import Redis from "ioredis" +import Redis, { Cluster } from "ioredis" // mock-redis doesn't have any typing let MockRedis: any | undefined if (env.MOCK_REDIS) { @@ -28,7 +28,7 @@ const DEFAULT_SELECT_DB = SelectableDatabase.DEFAULT // for testing just generate the client once let CLOSED = false -const CLIENTS: Record = {} +const CLIENTS: Record = {} let CONNECTED = false // mock redis always connected @@ -201,12 +201,15 @@ class RedisWrapper { key = `${db}${SEPARATOR}${key}` let stream if (CLUSTERED) { - let node = this.getClient().nodes("master") + let node = (this.getClient() as Cluster).nodes("master") stream = node[0].scanStream({ match: key + "*", count: 100 }) } else { - stream = this.getClient().scanStream({ match: key + "*", count: 100 }) + stream = (this.getClient() as Redis).scanStream({ + match: key + "*", + count: 100, + }) } - return promisifyStream(stream, this.getClient()) + return promisifyStream(stream, this.getClient() as any) } async keys(pattern: string) { @@ -221,14 +224,16 @@ class RedisWrapper { async get(key: string) { const db = this._db - let response = await this.getClient().get(addDbPrefix(db, key)) + const response = await this.getClient().get(addDbPrefix(db, key)) // overwrite the prefixed key + // @ts-ignore if (response != null && response.key) { + // @ts-ignore response.key = key } // if its not an object just return the response try { - return JSON.parse(response) + return JSON.parse(response!) } catch (err) { return response } @@ -280,7 +285,7 @@ class RedisWrapper { return this.getClient().ttl(prefixedKey) } - async setExpiry(key: string, expirySeconds: number | null) { + async setExpiry(key: string, expirySeconds: number) { const db = this._db const prefixedKey = addDbPrefix(db, key) await this.getClient().expire(prefixedKey, expirySeconds) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 7009dc6f55..adeb5b12ec 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -72,7 +72,7 @@ const OPTIONS: Record = { export async function newRedlock(opts: Redlock.Options = {}) { const options = { ...OPTIONS.DEFAULT, ...opts } const redisWrapper = await getLockClient() - const client = redisWrapper.getClient() + const client = redisWrapper.getClient() as any return new Redlock([client], options) } From a4288a9dd3e21dd99c3ac825effa7b1a40dadb63 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 16:41:49 +0100 Subject: [PATCH 04/14] Basic test --- .../src/redis/tests/redis.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 packages/backend-core/src/redis/tests/redis.spec.ts diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts new file mode 100644 index 0000000000..d082b6b617 --- /dev/null +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -0,0 +1,21 @@ +import { generator, structures } from "../../../tests" +import RedisWrapper from "../redis" + +describe("redis", () => { + const redis = new RedisWrapper(structures.db.id()) + + beforeAll(async () => { + await redis.init() + }) + + describe("store", () => { + it("a basic value can be persisted", async () => { + const key = structures.uuid() + const value = generator.word() + + await redis.store(key, value) + + expect(await redis.get(key)).toEqual(value) + }) + }) +}) From 49db47e1fd75b63b7bf06794e5418a54c266db3d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 16:46:33 +0100 Subject: [PATCH 05/14] Add bulk store --- packages/backend-core/src/redis/redis.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index f8f0c9f3d7..076f64b1ea 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -279,6 +279,19 @@ class RedisWrapper { } } + async bulkStore( + data: Record, + expirySeconds: number | null = null + ) { + const client = this.getClient() + + const dataToStore = Object.entries(data).reduce((acc, [key, value]) => { + acc[addDbPrefix(this._db, key)] = value + return acc + }, {} as Record) + await client.mset(dataToStore) + } + async getTTL(key: string) { const db = this._db const prefixedKey = addDbPrefix(db, key) From d9a5899b2770e7140c360f0eaf24b9e3ccb84d07 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 16:49:45 +0100 Subject: [PATCH 06/14] Bulk store test --- .../src/redis/tests/redis.spec.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index d082b6b617..6333573e6e 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -2,9 +2,10 @@ import { generator, structures } from "../../../tests" import RedisWrapper from "../redis" describe("redis", () => { - const redis = new RedisWrapper(structures.db.id()) + let redis: RedisWrapper - beforeAll(async () => { + beforeEach(async () => { + redis = new RedisWrapper(structures.db.id()) await redis.init() }) @@ -18,4 +19,23 @@ describe("redis", () => { expect(await redis.get(key)).toEqual(value) }) }) + + describe("bulkStore", () => { + it("a basic object can be persisted", async () => { + const data = generator + .unique(() => generator.word(), 10) + .reduce((acc, key) => { + acc[key] = generator.word() + return acc + }, {} as Record) + + await redis.bulkStore(data) + + for (const [key, value] of Object.entries(data)) { + expect(await redis.get(key)).toEqual(value) + } + + expect(await redis.keys("*")).toHaveLength(10) + }) + }) }) From 1b0a943e13fcb46cc91be18f794c3ff2a4c95684 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 17:04:46 +0100 Subject: [PATCH 07/14] Atomic expires --- packages/backend-core/src/redis/redis.ts | 19 ++++++++++++++++++- .../src/redis/tests/redis.spec.ts | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 076f64b1ea..18152aac72 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -289,7 +289,24 @@ class RedisWrapper { acc[addDbPrefix(this._db, key)] = value return acc }, {} as Record) - await client.mset(dataToStore) + + const luaScript = ` + for i, key in ipairs(KEYS) do + redis.call('MSET', key, ARGV[i]) + ${ + expirySeconds !== null + ? `redis.call('EXPIRE', key, ARGV[#ARGV])` + : "" + } + end + ` + const keys = Object.keys(dataToStore) + let values = Object.values(dataToStore) + if (expirySeconds !== null) { + values.push(expirySeconds) + } + + await client.eval(luaScript, keys.length, ...keys, ...values) } async getTTL(key: string) { diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index 6333573e6e..e3e4ae7247 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -37,5 +37,24 @@ describe("redis", () => { expect(await redis.keys("*")).toHaveLength(10) }) + + it("a bulk store can be persisted with TTL", async () => { + const ttl = 500 + const data = generator + .unique(() => generator.word(), 10) + .reduce((acc, key) => { + acc[key] = generator.word() + return acc + }, {} as Record) + + await redis.bulkStore(data, ttl) + + for (const [key, value] of Object.entries(data)) { + expect(await redis.get(key)).toEqual(value) + expect(await redis.getTTL(key)).toEqual(ttl) + } + + expect(await redis.keys("*")).toHaveLength(10) + }) }) }) From 3baf981d4826c68360261a1c57c9a97c1a4ed267 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 17:08:08 +0100 Subject: [PATCH 08/14] Add TTL tests --- .../src/redis/tests/redis.spec.ts | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index e3e4ae7247..13f2c676c9 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -21,13 +21,17 @@ describe("redis", () => { }) describe("bulkStore", () => { - it("a basic object can be persisted", async () => { - const data = generator - .unique(() => generator.word(), 10) + function createRandomObject(keyLength: number) { + return generator + .unique(() => generator.word(), keyLength) .reduce((acc, key) => { acc[key] = generator.word() return acc }, {} as Record) + } + + it("a basic object can be persisted", async () => { + const data = createRandomObject(10) await redis.bulkStore(data) @@ -38,14 +42,20 @@ describe("redis", () => { expect(await redis.keys("*")).toHaveLength(10) }) + it("no TTL is set by default", async () => { + const data = createRandomObject(10) + + await redis.bulkStore(data) + + for (const [key, value] of Object.entries(data)) { + expect(await redis.get(key)).toEqual(value) + expect(await redis.getTTL(key)).toEqual(-1) + } + }) + it("a bulk store can be persisted with TTL", async () => { const ttl = 500 - const data = generator - .unique(() => generator.word(), 10) - .reduce((acc, key) => { - acc[key] = generator.word() - return acc - }, {} as Record) + const data = createRandomObject(8) await redis.bulkStore(data, ttl) @@ -54,7 +64,20 @@ describe("redis", () => { expect(await redis.getTTL(key)).toEqual(ttl) } - expect(await redis.keys("*")).toHaveLength(10) + expect(await redis.keys("*")).toHaveLength(8) + }) + + it("setting a TTL of -1 will not persist the key", async () => { + const ttl = -1 + const data = createRandomObject(5) + + await redis.bulkStore(data, ttl) + + for (const [key, value] of Object.entries(data)) { + expect(await redis.get(key)).toBe(null) + } + + expect(await redis.keys("*")).toHaveLength(0) }) }) }) From 8f9e8b60c328af4f635c98759b475f08668d7eed Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 17:19:26 +0100 Subject: [PATCH 09/14] Fix types --- packages/backend-core/src/redis/redis.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 18152aac72..99613e7c32 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -28,7 +28,7 @@ const DEFAULT_SELECT_DB = SelectableDatabase.DEFAULT // for testing just generate the client once let CLOSED = false -const CLIENTS: Record = {} +const CLIENTS: Record = {} let CONNECTED = false // mock redis always connected @@ -201,7 +201,7 @@ class RedisWrapper { key = `${db}${SEPARATOR}${key}` let stream if (CLUSTERED) { - let node = (this.getClient() as Cluster).nodes("master") + let node = (this.getClient() as never as Cluster).nodes("master") stream = node[0].scanStream({ match: key + "*", count: 100 }) } else { stream = (this.getClient() as Redis).scanStream({ From f2330144de5bfdddc3d9b4021540e0cc9976c3ca Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 17:57:28 +0100 Subject: [PATCH 10/14] Clean --- packages/backend-core/src/redis/redis.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 99613e7c32..59583da366 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -301,7 +301,7 @@ class RedisWrapper { end ` const keys = Object.keys(dataToStore) - let values = Object.values(dataToStore) + const values = Object.values(dataToStore) if (expirySeconds !== null) { values.push(expirySeconds) } From 82ff748fd950e79e60daad95268d5c34490d25f7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 18:10:33 +0100 Subject: [PATCH 11/14] Add complex object tests --- .../src/redis/tests/redis.spec.ts | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index 13f2c676c9..eb32172edd 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -18,14 +18,26 @@ describe("redis", () => { expect(await redis.get(key)).toEqual(value) }) + + it("objects can be persisted", async () => { + const key = structures.uuid() + const value = { [generator.word()]: generator.word() } + + await redis.store(key, value) + + expect(await redis.get(key)).toEqual(value) + }) }) describe("bulkStore", () => { - function createRandomObject(keyLength: number) { + function createRandomObject( + keyLength: number, + valueGenerator: () => any = () => generator.word() + ) { return generator .unique(() => generator.word(), keyLength) .reduce((acc, key) => { - acc[key] = generator.word() + acc[key] = valueGenerator() return acc }, {} as Record) } @@ -42,6 +54,21 @@ describe("redis", () => { expect(await redis.keys("*")).toHaveLength(10) }) + it("a complex object can be persisted", async () => { + const data = { + ...createRandomObject(10, () => createRandomObject(5)), + ...createRandomObject(5), + } + + await redis.bulkStore(data) + + for (const [key, value] of Object.entries(data)) { + expect(await redis.get(key)).toEqual(value) + } + + expect(await redis.keys("*")).toHaveLength(10) + }) + it("no TTL is set by default", async () => { const data = createRandomObject(10) From de0527384aebd2703fd7eec58818366627da8b10 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 18:10:45 +0100 Subject: [PATCH 12/14] Support complex objects --- packages/backend-core/src/redis/redis.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 59583da366..8cfa3db5c1 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -286,7 +286,8 @@ class RedisWrapper { const client = this.getClient() const dataToStore = Object.entries(data).reduce((acc, [key, value]) => { - acc[addDbPrefix(this._db, key)] = value + acc[addDbPrefix(this._db, key)] = + typeof value === "object" ? JSON.stringify(value) : value return acc }, {} as Record) From a093cfca993d585ae5a83e933dcacccee35c4bf5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 5 Mar 2024 18:11:12 +0100 Subject: [PATCH 13/14] Fix test --- packages/backend-core/src/redis/tests/redis.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/redis/tests/redis.spec.ts b/packages/backend-core/src/redis/tests/redis.spec.ts index eb32172edd..1fd40acc37 100644 --- a/packages/backend-core/src/redis/tests/redis.spec.ts +++ b/packages/backend-core/src/redis/tests/redis.spec.ts @@ -66,7 +66,7 @@ describe("redis", () => { expect(await redis.get(key)).toEqual(value) } - expect(await redis.keys("*")).toHaveLength(10) + expect(await redis.keys("*")).toHaveLength(15) }) it("no TTL is set by default", async () => { From 4baadadaa8d2572617236ebb26d66cdbde88f611 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 6 Mar 2024 12:22:20 +0100 Subject: [PATCH 14/14] Use pipeline instead of eval --- packages/backend-core/src/redis/redis.ts | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 8cfa3db5c1..6124f5f447 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -291,23 +291,16 @@ class RedisWrapper { return acc }, {} as Record) - const luaScript = ` - for i, key in ipairs(KEYS) do - redis.call('MSET', key, ARGV[i]) - ${ - expirySeconds !== null - ? `redis.call('EXPIRE', key, ARGV[#ARGV])` - : "" - } - end - ` - const keys = Object.keys(dataToStore) - const values = Object.values(dataToStore) + const pipeline = client.pipeline() + pipeline.mset(dataToStore) + if (expirySeconds !== null) { - values.push(expirySeconds) + for (const key of Object.keys(dataToStore)) { + pipeline.expire(key, expirySeconds) + } } - await client.eval(luaScript, keys.length, ...keys, ...values) + await pipeline.exec() } async getTTL(key: string) {