diff --git a/libs/shared/db/mysql/account/src/lib/setup.ts b/libs/shared/db/mysql/account/src/lib/setup.ts index 880f2dc729b..a5a3aff3e76 100644 --- a/libs/shared/db/mysql/account/src/lib/setup.ts +++ b/libs/shared/db/mysql/account/src/lib/setup.ts @@ -10,8 +10,29 @@ export type AccountDatabase = Kysely; export const AccountDbProvider = Symbol('AccountDbProvider'); export async function setupAccountDatabase(opts: MySQLConfig) { - const dialect = await createDialect(opts); - return new Kysely({ + const { dialect, pool } = await createDialect(opts); + const db = new Kysely({ dialect, }); + + /** + * Important! We've observed that connection pools aren't being destroyed. + * In theory Kysely should do this when the instance is destroyed, but it + * appears not work in practice. The following hijack works around the + * issue and manually closes the connection pool we created. + * + * This was most noticeable when running auth-server remote tests, where + * we'd hit random timeouts acquiring connections because previous connections + * pools were never closed. + * + * When running our services in production, this isn't really a noticeable + * because we aren't creating lots of new account database instances. + */ + const _dbDestroy = db.destroy; + db.destroy = async function () { + await _dbDestroy.call(db); + pool.end(() => {}); + }; + + return db; } diff --git a/libs/shared/db/mysql/core/src/lib/core.ts b/libs/shared/db/mysql/core/src/lib/core.ts index 5a603c6bbb0..b77494cf5f2 100644 --- a/libs/shared/db/mysql/core/src/lib/core.ts +++ b/libs/shared/db/mysql/core/src/lib/core.ts @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { knex, Knex } from 'knex'; -import { MysqlDialect } from 'kysely'; +import { MysqlDialect, MysqlPool } from 'kysely'; import { createPool } from 'mysql2'; import { promisify } from 'util'; import { v4 as uuidv4 } from 'uuid'; @@ -113,7 +113,10 @@ export async function createDialect( opts: MySQLConfig, log: Logger = logger, metrics: StatsD = localStatsD() -) { +): Promise<{ + dialect: MysqlDialect; + pool: MysqlPool; +}> { log.debug('mysqlDialect', { msg: `mysqlDialect: Creating MysqlDialect`, connLimit: opts.connectionLimitMax, @@ -137,6 +140,9 @@ export async function createDialect( pool.on('release', (eventId: any) => { metrics.increment('mysqlDialect.release'); }); + pool.on('enqueue', (eventId: any) => { + metrics.increment('mysqlDialect.enqueue'); + }); // Connection setup // Always communicate timestamps in UTC. @@ -165,7 +171,14 @@ export async function createDialect( await pool.promise().query("SET SESSION sql_mode = '" + mode + "'"); } - return new MysqlDialect({ + const dialect = new MysqlDialect({ pool, }); + + // Important! Exposing the underlying pool, because + // Kysely is not automatically disposing it. + return { + dialect, + pool, + }; } diff --git a/packages/fxa-auth-server/bin/key_server.js b/packages/fxa-auth-server/bin/key_server.js index fcc20634717..3e460e6bd83 100755 --- a/packages/fxa-auth-server/bin/key_server.js +++ b/packages/fxa-auth-server/bin/key_server.js @@ -395,6 +395,15 @@ async function run(config) { message: 'Database connection did not shutdown cleanly. ' + e.message, }); } + + try { + await accountDatabase.destroy(); + } catch (e) { + log.warn('shutdown', { + message: + 'Account database connection did not shutdown cleanly.' + e.message, + }); + } }, }; } diff --git a/packages/fxa-auth-server/test/remote/recovery_phone_tests.js b/packages/fxa-auth-server/test/remote/recovery_phone_tests.js index a147ccb1a0f..3179850434b 100644 --- a/packages/fxa-auth-server/test/remote/recovery_phone_tests.js +++ b/packages/fxa-auth-server/test/remote/recovery_phone_tests.js @@ -110,6 +110,7 @@ describe(`#integration - recovery phone`, function () { after(async () => { await TestServer.stop(server); + await db.destroy(); }); it('sets up a recovery phone', async function () {