From 125082f461a435dfdfbaded0bf926a47338bd851 Mon Sep 17 00:00:00 2001 From: Fabrizio Date: Mon, 6 Nov 2023 14:40:05 +0000 Subject: [PATCH] fix: lru cache on internal pool (#392) --- package-lock.json | 49 ++++++------------- package.json | 2 +- src/database/connection.ts | 24 ++++++--- .../events/multipart-upload-completed.ts | 20 +++++++- src/queue/events/object-admin-delete.ts | 20 +++++++- src/storage/database/adapter.ts | 2 + src/storage/database/knex.ts | 4 ++ 7 files changed, 75 insertions(+), 46 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7425bfcf..0ac45368 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "@fastify/rate-limit": "^7.6.0", "@fastify/swagger": "^8.3.1", "@fastify/swagger-ui": "^1.7.0", + "@isaacs/ttlcache": "^1.4.1", "@tus/file-store": "^1.0.0-beta.1", "@tus/s3-store": "https://gitpkg.now.sh/supabase/tus-node-server/packages/s3-store/dist?build", "@tus/server": "https://gitpkg.now.sh/supabase/tus-node-server/packages/server/dist?build", @@ -57,7 +58,6 @@ "@types/jest": "^29.2.1", "@types/js-yaml": "^4.0.5", "@types/jsonwebtoken": "^8.5.8", - "@types/lru-cache": "^7.10.10", "@types/mustache": "^4.2.2", "@types/node": "^18.14.6", "@types/pg": "^8.6.4", @@ -2658,6 +2658,14 @@ "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.2.0.tgz", "integrity": "sha512-Sx1pU8EM64o2BrqNpEO1CNLtKQwyhuXuqyfH7oGKCk+1a33d2r5saW8zNwm3j6BTExtjrv2BxTgzzkMwts6vGg==" }, + "node_modules/@isaacs/ttlcache": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/@isaacs/ttlcache/-/ttlcache-1.4.1.tgz", + "integrity": "sha512-RQgQ4uQ+pLbqXfOmieB91ejmLwvSgv9nLx6sT6sD83s7umBypgg+OIBOBbEUiJXrfpnp9j0mRhYYdzp9uqq3lA==", + "engines": { + "node": ">=12" + } + }, "node_modules/@istanbuljs/load-nyc-config": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz", @@ -3920,16 +3928,6 @@ "@types/node": "*" } }, - "node_modules/@types/lru-cache": { - "version": "7.10.10", - "resolved": "https://registry.npmjs.org/@types/lru-cache/-/lru-cache-7.10.10.tgz", - "integrity": "sha512-nEpVRPWW9EBmx2SCfNn3ClYxPL7IktPX12HhIoSc/H5mMjdeW3+YsXIpseLQ2xF35+OcpwKQbEUw5VtqE4PDNA==", - "deprecated": "This is a stub types definition. lru-cache provides its own type definitions, so you do not need this installed.", - "dev": true, - "dependencies": { - "lru-cache": "*" - } - }, "node_modules/@types/mustache": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/@types/mustache/-/mustache-4.2.2.tgz", @@ -7734,15 +7732,6 @@ "resolved": "https://registry.npmjs.org/logflare-transport-core/-/logflare-transport-core-0.3.3.tgz", "integrity": "sha512-n82NsRVWvlaa3jd9QQ8rDroCjCJcIamQOlarLDBou9RsF0QaRv39rduy0ToPmlGQn1OPZBwlsv+R36lXupSmVQ==" }, - "node_modules/lru-cache": { - "version": "7.17.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-7.17.0.tgz", - "integrity": "sha512-zSxlVVwOabhVyTi6E8gYv2cr6bXK+8ifYz5/uyJb9feXX6NACVDwY4p5Ut3WC3Ivo/QhpARHU3iujx2xGAYHbQ==", - "dev": true, - "engines": { - "node": ">=12" - } - }, "node_modules/luxon": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/luxon/-/luxon-3.2.1.tgz", @@ -12160,6 +12149,11 @@ "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.2.0.tgz", "integrity": "sha512-Sx1pU8EM64o2BrqNpEO1CNLtKQwyhuXuqyfH7oGKCk+1a33d2r5saW8zNwm3j6BTExtjrv2BxTgzzkMwts6vGg==" }, + "@isaacs/ttlcache": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/@isaacs/ttlcache/-/ttlcache-1.4.1.tgz", + "integrity": "sha512-RQgQ4uQ+pLbqXfOmieB91ejmLwvSgv9nLx6sT6sD83s7umBypgg+OIBOBbEUiJXrfpnp9j0mRhYYdzp9uqq3lA==" + }, "@istanbuljs/load-nyc-config": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz", @@ -13197,15 +13191,6 @@ "@types/node": "*" } }, - "@types/lru-cache": { - "version": "7.10.10", - "resolved": "https://registry.npmjs.org/@types/lru-cache/-/lru-cache-7.10.10.tgz", - "integrity": "sha512-nEpVRPWW9EBmx2SCfNn3ClYxPL7IktPX12HhIoSc/H5mMjdeW3+YsXIpseLQ2xF35+OcpwKQbEUw5VtqE4PDNA==", - "dev": true, - "requires": { - "lru-cache": "*" - } - }, "@types/mustache": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/@types/mustache/-/mustache-4.2.2.tgz", @@ -16050,12 +16035,6 @@ "resolved": "https://registry.npmjs.org/logflare-transport-core/-/logflare-transport-core-0.3.3.tgz", "integrity": "sha512-n82NsRVWvlaa3jd9QQ8rDroCjCJcIamQOlarLDBou9RsF0QaRv39rduy0ToPmlGQn1OPZBwlsv+R36lXupSmVQ==" }, - "lru-cache": { - "version": "7.17.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-7.17.0.tgz", - "integrity": "sha512-zSxlVVwOabhVyTi6E8gYv2cr6bXK+8ifYz5/uyJb9feXX6NACVDwY4p5Ut3WC3Ivo/QhpARHU3iujx2xGAYHbQ==", - "dev": true - }, "luxon": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/luxon/-/luxon-3.2.1.tgz", diff --git a/package.json b/package.json index 3a26a82d..a5db04b1 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "@fastify/rate-limit": "^7.6.0", "@fastify/swagger": "^8.3.1", "@fastify/swagger-ui": "^1.7.0", + "@isaacs/ttlcache": "^1.4.1", "@tus/file-store": "^1.0.0-beta.1", "@tus/s3-store": "https://gitpkg.now.sh/supabase/tus-node-server/packages/s3-store/dist?build", "@tus/server": "https://gitpkg.now.sh/supabase/tus-node-server/packages/server/dist?build", @@ -70,7 +71,6 @@ "@types/jest": "^29.2.1", "@types/js-yaml": "^4.0.5", "@types/jsonwebtoken": "^8.5.8", - "@types/lru-cache": "^7.10.10", "@types/mustache": "^4.2.2", "@types/node": "^18.14.6", "@types/pg": "^8.6.4", diff --git a/src/database/connection.ts b/src/database/connection.ts index aada5c2d..f6fab637 100644 --- a/src/database/connection.ts +++ b/src/database/connection.ts @@ -2,6 +2,7 @@ import pg, { DatabaseError } from 'pg' import { Knex, knex } from 'knex' import { JwtPayload } from 'jsonwebtoken' import retry from 'async-retry' +import TTLCache from '@isaacs/ttlcache' import { getConfig } from '../config' import { DbActiveConnection, DbActivePool } from '../monitoring/metrics' import { StorageBackendError } from '../storage' @@ -11,6 +12,7 @@ import KnexTimeoutError = knex.KnexTimeoutError pg.types.setTypeParser(20, 'text', parseInt) const { + isMultitenant, databaseSSLRootCert, databaseMaxConnections, databaseFreePoolAfterInactivity, @@ -35,7 +37,19 @@ export interface User { payload: { role?: string } & JwtPayload } -export const connections = new Map() +const multiTenantLRUConfig = { + ttl: 1000 * 10, + updateAgeOnGet: true, + checkAgeOnGet: true, +} +export const connections = new TTLCache({ + ...(isMultitenant ? multiTenantLRUConfig : { max: 1, ttl: Infinity }), + dispose: async (pool) => { + if (!pool) return + await pool.destroy() + pool.client.removeAllListeners() + }, +}) const searchPath = ['storage', 'public', 'extensions'] export class TenantConnection { @@ -50,10 +64,12 @@ export class TenantConnection { static stop() { const promises: Promise[] = [] + for (const [connectionString, pool] of connections) { promises.push(pool.destroy()) connections.delete(connectionString) } + return Promise.allSettled(promises) } @@ -104,12 +120,6 @@ export class TenantConnection { }) if (!isExternalPool) { - knexPool.client.pool.on('poolDestroySuccess', () => { - if (connections.get(connectionString) === knexPool) { - connections.delete(connectionString) - } - }) - connections.set(connectionString, knexPool) } diff --git a/src/queue/events/multipart-upload-completed.ts b/src/queue/events/multipart-upload-completed.ts index 61d4e6d1..23c61de6 100644 --- a/src/queue/events/multipart-upload-completed.ts +++ b/src/queue/events/multipart-upload-completed.ts @@ -2,7 +2,7 @@ import { BaseEvent, BasePayload } from './base-event' import { Job } from 'pg-boss' import { getConfig } from '../../config' import { S3Backend } from '../../storage/backend' -import { isS3Error } from '../../storage' +import { isS3Error, Storage } from '../../storage' import { logger } from '../../monitoring' interface UploadCompleted extends BasePayload { @@ -17,8 +17,9 @@ export class MultiPartUploadCompleted extends BaseEvent { static queueName = 'multipart:upload:completed' static async handle(job: Job) { + let storage: Storage | undefined = undefined try { - const storage = await this.createStorage(job.data) + storage = await this.createStorage(job.data) const version = job.data.version const s3Key = `${job.data.tenant.ref}/${job.data.bucketName}/${job.data.objectName}/${version}` @@ -32,6 +33,21 @@ export class MultiPartUploadCompleted extends BaseEvent { } logger.error({ error: e }, 'multi part uploaded completed failed') throw e + } finally { + if (storage) { + const tenant = storage.db.tenant() + storage.db + .destroyConnection() + .then(() => { + // no-op + }) + .catch((e) => { + logger.error( + { error: e }, + `[Admin]: MultiPartUploadCompleted ${tenant.ref} - FAILED DISPOSING CONNECTION` + ) + }) + } } } } diff --git a/src/queue/events/object-admin-delete.ts b/src/queue/events/object-admin-delete.ts index 2f88dfad..8b67a060 100644 --- a/src/queue/events/object-admin-delete.ts +++ b/src/queue/events/object-admin-delete.ts @@ -3,6 +3,7 @@ import { getConfig } from '../../config' import { Job, WorkOptions } from 'pg-boss' import { withOptionalVersion } from '../../storage/backend' import { logger, logSchema } from '../../monitoring' +import { Storage } from '../../storage' export interface ObjectDeleteEvent extends BasePayload { name: string @@ -23,8 +24,10 @@ export class ObjectAdminDelete extends BaseEvent { } static async handle(job: Job) { + let storage: Storage | undefined = undefined + try { - const storage = await this.createStorage(job.data) + storage = await this.createStorage(job.data) const version = job.data.version const s3Key = `${job.data.tenant.ref}/${job.data.bucketId}/${job.data.name}` @@ -62,6 +65,21 @@ export class ObjectAdminDelete extends BaseEvent { `[Admin]: ObjectAdminDelete ${s3Key} - FAILED` ) throw e + } finally { + if (storage) { + const tenant = storage.db.tenant() + storage.db + .destroyConnection() + .then(() => { + // no-op + }) + .catch((e) => { + logger.error( + { error: e }, + `[Admin]: ObjectAdminDelete ${tenant.ref} - FAILED DISPOSING CONNECTION` + ) + }) + } } } } diff --git a/src/storage/database/adapter.ts b/src/storage/database/adapter.ts index 55c71c53..f34cb742 100644 --- a/src/storage/database/adapter.ts +++ b/src/storage/database/adapter.ts @@ -118,4 +118,6 @@ export interface Database { searchObjects(bucketId: string, prefix: string, options: SearchObjectOption): Promise healthcheck(): Promise + + destroyConnection(): Promise } diff --git a/src/storage/database/knex.ts b/src/storage/database/knex.ts index f813a45e..3d4da899 100644 --- a/src/storage/database/knex.ts +++ b/src/storage/database/knex.ts @@ -463,6 +463,10 @@ export class StorageKnexDB implements Database { }) } + destroyConnection() { + return this.connection.dispose() + } + protected async runQuery Promise>( queryName: string, fn: T