From 828399ec14b1535b37719621189441418b817a28 Mon Sep 17 00:00:00 2001 From: Jonas Lochmann Date: Mon, 8 Mar 2021 01:00:00 +0100 Subject: [PATCH] Improve handling of exceptions which should trigger retrying an operation --- scripts/util/mainapp.js | 2 +- src/database/config.ts | 3 +- src/database/index.ts | 132 +--------------------- src/database/main.ts | 101 +++++++++++++++++ src/database/utils/nested-transactions.ts | 62 ++++++++++ src/database/utils/serialized.ts | 106 +++++++++++++++++ src/function/sync/apply-actions/index.ts | 8 +- src/index.ts | 5 +- 8 files changed, 285 insertions(+), 134 deletions(-) create mode 100644 src/database/main.ts create mode 100644 src/database/utils/nested-transactions.ts create mode 100644 src/database/utils/serialized.ts diff --git a/scripts/util/mainapp.js b/scripts/util/mainapp.js index 8112af2..0d141f9 100644 --- a/scripts/util/mainapp.js +++ b/scripts/util/mainapp.js @@ -24,7 +24,7 @@ function startMainApp(env) { return new Promise((resolve, reject) => { const task = spawn('node', [initPath], { stdio: ['inherit', 'pipe', 'inherit'], - env: { ...process.env, PORT: 0 /* random port */ } + env: { ...process.env, PORT: 0 /* random port */, ...env } }) task.on('exit', () => reject(new Error('task terminated too early'))) diff --git a/src/database/config.ts b/src/database/config.ts index c4580ec..1563102 100644 --- a/src/database/config.ts +++ b/src/database/config.ts @@ -47,5 +47,6 @@ export const createConfigModel = (sequelize: Sequelize.Sequelize): ConfigModelSt export const configItemIds = { statusMessage: 'status_message', - selfTestData: 'self_test_data' + selfTestData: 'self_test_data', + secondSelfTestData: 'self_test_data_two' } diff --git a/src/database/index.ts b/src/database/index.ts index 945c412..d1e5813 100644 --- a/src/database/index.ts +++ b/src/database/index.ts @@ -1,6 +1,6 @@ /* * server component for the TimeLimit App - * Copyright (C) 2019 - 2020 Jonas Lochmann + * Copyright (C) 2019 - 2021 Jonas Lochmann * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -15,130 +15,6 @@ * along with this program. If not, see . */ -import * as Sequelize from 'sequelize' -import { generateIdWithinFamily } from '../util/token' -import { AddDeviceTokenModelStatic, createAddDeviceTokenModel } from './adddevicetoken' -import { AppModelStatic, createAppModel } from './app' -import { AppActivityModelStatic, createAppActivityModel } from './appactivity' -import { AuthTokenModelStatic, createAuthtokenModel } from './authtoken' -import { CategoryModelStatic, createCategoryModel } from './category' -import { CategoryAppModelStatic, createCategoryAppModel } from './categoryapp' -import { CategoryNetworkIdModelStatic, createCategoryNetworkIdModel } from './categorynetworkid' -import { ChildTaskModelStatic, createChildTaskModel } from './childtask' -import { configItemIds, ConfigModelStatic, createConfigModel } from './config' -import { createDeviceModel, DeviceModelStatic } from './device' -import { createFamilyModel, FamilyModelStatic } from './family' -import { createMailLoginTokenModel, MailLoginTokenModelStatic } from './maillogintoken' -import { createUmzug } from './migration/umzug' -import { createOldDeviceModel, OldDeviceModelStatic } from './olddevice' -import { createPurchaseModel, PurchaseModelStatic } from './purchase' -import { createSessionDurationModel, SessionDurationModelStatic } from './sessionduration' -import { createTimelimitRuleModel, TimelimitRuleModelStatic } from './timelimitrule' -import { createUsedTimeModel, UsedTimeModelStatic } from './usedtime' -import { createUserModel, UserModelStatic } from './user' -import { createUserLimitLoginCategoryModel, UserLimitLoginCategoryModelStatic } from './userlimitlogincategory' - -export type Transaction = Sequelize.Transaction - -export interface Database { - addDeviceToken: AddDeviceTokenModelStatic - authtoken: AuthTokenModelStatic - app: AppModelStatic - appActivity: AppActivityModelStatic - category: CategoryModelStatic - categoryApp: CategoryAppModelStatic - categoryNetworkId: CategoryNetworkIdModelStatic - childTask: ChildTaskModelStatic - config: ConfigModelStatic - device: DeviceModelStatic - family: FamilyModelStatic - mailLoginToken: MailLoginTokenModelStatic - oldDevice: OldDeviceModelStatic - purchase: PurchaseModelStatic - sessionDuration: SessionDurationModelStatic - timelimitRule: TimelimitRuleModelStatic - usedTime: UsedTimeModelStatic - user: UserModelStatic - userLimitLoginCategory: UserLimitLoginCategoryModelStatic - transaction: (autoCallback: (t: Transaction) => Promise, options?: { transaction: Transaction }) => Promise - dialect: string -} - -const createDatabase = (sequelize: Sequelize.Sequelize): Database => ({ - addDeviceToken: createAddDeviceTokenModel(sequelize), - authtoken: createAuthtokenModel(sequelize), - app: createAppModel(sequelize), - appActivity: createAppActivityModel(sequelize), - category: createCategoryModel(sequelize), - categoryApp: createCategoryAppModel(sequelize), - childTask: createChildTaskModel(sequelize), - categoryNetworkId: createCategoryNetworkIdModel(sequelize), - config: createConfigModel(sequelize), - device: createDeviceModel(sequelize), - family: createFamilyModel(sequelize), - mailLoginToken: createMailLoginTokenModel(sequelize), - oldDevice: createOldDeviceModel(sequelize), - purchase: createPurchaseModel(sequelize), - sessionDuration: createSessionDurationModel(sequelize), - timelimitRule: createTimelimitRuleModel(sequelize), - usedTime: createUsedTimeModel(sequelize), - user: createUserModel(sequelize), - userLimitLoginCategory: createUserLimitLoginCategoryModel(sequelize), - transaction: (autoCallback: (transaction: Transaction) => Promise, options?: { transaction: Transaction }) => (sequelize.transaction({ - isolationLevel: Sequelize.Transaction.ISOLATION_LEVELS.READ_COMMITTED, - transaction: options?.transaction - }, autoCallback) as any) as Promise, - dialect: sequelize.getDialect() -}) - -export const sequelize = new Sequelize.Sequelize(process.env.DATABASE_URL || 'sqlite://test.db', { - define: { - timestamps: false - }, - logging: false -}) - -export const defaultDatabase = createDatabase(sequelize) -export const defaultUmzug = createUmzug(sequelize) - -class NestedTransactionTestException extends Error {} -class TestRollbackException extends NestedTransactionTestException {} -class NestedTransactionsNotWorkingException extends NestedTransactionTestException { constructor () { super('NestedTransactionsNotWorkingException') } } -class IllegalStateException extends NestedTransactionTestException {} - -export async function assertNestedTransactionsAreWorking (database: Database) { - const testValue = generateIdWithinFamily() - - // clean up just for the case - await database.config.destroy({ where: { id: configItemIds.selfTestData } }) - - await database.transaction(async (transaction) => { - const readOne = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) - - if (readOne) throw new IllegalStateException() - - await database.transaction(async (transaction) => { - await database.config.create({ id: configItemIds.selfTestData, value: testValue }, { transaction }) - - const readTwo = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) - - if (readTwo?.value !== testValue) throw new IllegalStateException() - - try { - await database.transaction(async (transaction) => { - await database.config.destroy({ where: { id: configItemIds.selfTestData }, transaction }) - - throw new TestRollbackException() - }, { transaction }) - } catch (ex) { - if (!(ex instanceof TestRollbackException)) throw ex - } - - const readThree = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) - - if (readThree?.value !== testValue) throw new NestedTransactionsNotWorkingException() - - await database.config.destroy({ where: { id: configItemIds.selfTestData }, transaction }) - }, { transaction }) - }) -} +export { Transaction, Database, defaultDatabase, defaultUmzug } from './main' +export { assertNestedTransactionsAreWorking } from './utils/nested-transactions' +export { assertSerializeableTransactionsAreWorking, shouldRetryWithException } from './utils/serialized' diff --git a/src/database/main.ts b/src/database/main.ts new file mode 100644 index 0000000..7dd6f93 --- /dev/null +++ b/src/database/main.ts @@ -0,0 +1,101 @@ +/* + * server component for the TimeLimit App + * Copyright (C) 2019 - 2021 Jonas Lochmann + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import * as Sequelize from 'sequelize' +import { AddDeviceTokenModelStatic, createAddDeviceTokenModel } from './adddevicetoken' +import { AppModelStatic, createAppModel } from './app' +import { AppActivityModelStatic, createAppActivityModel } from './appactivity' +import { AuthTokenModelStatic, createAuthtokenModel } from './authtoken' +import { CategoryModelStatic, createCategoryModel } from './category' +import { CategoryAppModelStatic, createCategoryAppModel } from './categoryapp' +import { CategoryNetworkIdModelStatic, createCategoryNetworkIdModel } from './categorynetworkid' +import { ChildTaskModelStatic, createChildTaskModel } from './childtask' +import { ConfigModelStatic, createConfigModel } from './config' +import { createDeviceModel, DeviceModelStatic } from './device' +import { createFamilyModel, FamilyModelStatic } from './family' +import { createMailLoginTokenModel, MailLoginTokenModelStatic } from './maillogintoken' +import { createUmzug } from './migration/umzug' +import { createOldDeviceModel, OldDeviceModelStatic } from './olddevice' +import { createPurchaseModel, PurchaseModelStatic } from './purchase' +import { createSessionDurationModel, SessionDurationModelStatic } from './sessionduration' +import { createTimelimitRuleModel, TimelimitRuleModelStatic } from './timelimitrule' +import { createUsedTimeModel, UsedTimeModelStatic } from './usedtime' +import { createUserModel, UserModelStatic } from './user' +import { createUserLimitLoginCategoryModel, UserLimitLoginCategoryModelStatic } from './userlimitlogincategory' + +export type Transaction = Sequelize.Transaction + +export interface Database { + addDeviceToken: AddDeviceTokenModelStatic + authtoken: AuthTokenModelStatic + app: AppModelStatic + appActivity: AppActivityModelStatic + category: CategoryModelStatic + categoryApp: CategoryAppModelStatic + categoryNetworkId: CategoryNetworkIdModelStatic + childTask: ChildTaskModelStatic + config: ConfigModelStatic + device: DeviceModelStatic + family: FamilyModelStatic + mailLoginToken: MailLoginTokenModelStatic + oldDevice: OldDeviceModelStatic + purchase: PurchaseModelStatic + sessionDuration: SessionDurationModelStatic + timelimitRule: TimelimitRuleModelStatic + usedTime: UsedTimeModelStatic + user: UserModelStatic + userLimitLoginCategory: UserLimitLoginCategoryModelStatic + transaction: (autoCallback: (t: Transaction) => Promise, options?: { transaction: Transaction }) => Promise + dialect: string +} + +const createDatabase = (sequelize: Sequelize.Sequelize): Database => ({ + addDeviceToken: createAddDeviceTokenModel(sequelize), + authtoken: createAuthtokenModel(sequelize), + app: createAppModel(sequelize), + appActivity: createAppActivityModel(sequelize), + category: createCategoryModel(sequelize), + categoryApp: createCategoryAppModel(sequelize), + childTask: createChildTaskModel(sequelize), + categoryNetworkId: createCategoryNetworkIdModel(sequelize), + config: createConfigModel(sequelize), + device: createDeviceModel(sequelize), + family: createFamilyModel(sequelize), + mailLoginToken: createMailLoginTokenModel(sequelize), + oldDevice: createOldDeviceModel(sequelize), + purchase: createPurchaseModel(sequelize), + sessionDuration: createSessionDurationModel(sequelize), + timelimitRule: createTimelimitRuleModel(sequelize), + usedTime: createUsedTimeModel(sequelize), + user: createUserModel(sequelize), + userLimitLoginCategory: createUserLimitLoginCategoryModel(sequelize), + transaction: (autoCallback: (transaction: Transaction) => Promise, options?: { transaction: Transaction }) => (sequelize.transaction({ + isolationLevel: Sequelize.Transaction.ISOLATION_LEVELS.SERIALIZABLE, + transaction: options?.transaction + }, autoCallback) as any) as Promise, + dialect: sequelize.getDialect() +}) + +export const sequelize = new Sequelize.Sequelize(process.env.DATABASE_URL || 'sqlite://test.db', { + define: { + timestamps: false + }, + logging: false +}) + +export const defaultDatabase = createDatabase(sequelize) +export const defaultUmzug = createUmzug(sequelize) diff --git a/src/database/utils/nested-transactions.ts b/src/database/utils/nested-transactions.ts new file mode 100644 index 0000000..a6b5239 --- /dev/null +++ b/src/database/utils/nested-transactions.ts @@ -0,0 +1,62 @@ +/* + * server component for the TimeLimit App + * Copyright (C) 2019 - 2021 Jonas Lochmann + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { generateIdWithinFamily } from '../../util/token' +import { configItemIds } from '../config' +import { Database } from '../main' + +class NestedTransactionTestException extends Error {} +class TestRollbackException extends NestedTransactionTestException {} +class NestedTransactionsNotWorkingException extends NestedTransactionTestException { constructor () { super('NestedTransactionsNotWorkingException') } } +class IllegalStateException extends NestedTransactionTestException {} + +export async function assertNestedTransactionsAreWorking (database: Database) { + const testValue = generateIdWithinFamily() + + // clean up just for the case + await database.config.destroy({ where: { id: configItemIds.selfTestData } }) + + await database.transaction(async (transaction) => { + const readOne = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) + + if (readOne) throw new IllegalStateException() + + await database.transaction(async (transaction) => { + await database.config.create({ id: configItemIds.selfTestData, value: testValue }, { transaction }) + + const readTwo = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) + + if (readTwo?.value !== testValue) throw new IllegalStateException() + + try { + await database.transaction(async (transaction) => { + await database.config.destroy({ where: { id: configItemIds.selfTestData }, transaction }) + + throw new TestRollbackException() + }, { transaction }) + } catch (ex) { + if (!(ex instanceof TestRollbackException)) throw ex + } + + const readThree = await database.config.findOne({ where: { id: configItemIds.selfTestData }, transaction }) + + if (readThree?.value !== testValue) throw new NestedTransactionsNotWorkingException() + + await database.config.destroy({ where: { id: configItemIds.selfTestData }, transaction }) + }, { transaction }) + }) +} diff --git a/src/database/utils/serialized.ts b/src/database/utils/serialized.ts new file mode 100644 index 0000000..7afa283 --- /dev/null +++ b/src/database/utils/serialized.ts @@ -0,0 +1,106 @@ +/* + * server component for the TimeLimit App + * Copyright (C) 2019 - 2021 Jonas Lochmann + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import * as Sequelize from 'sequelize' +import { configItemIds } from '../config' +import { Database } from '../main' + +export class SerializationFeatureCheckException extends Error {} + +export function shouldRetryWithException(database: Database, e: any): boolean { + if (e instanceof Sequelize.TimeoutError) return true + + if (!(e instanceof Sequelize.DatabaseError)) return false + + const parent = e.parent + + if (typeof parent !== 'object') return false + + if (database.dialect === 'sqlite') { + if (parent.message.startsWith('SQLITE_BUSY:')) return true + } else if (database.dialect === 'postgres') { + // 40001 = serialization_failure + if ((parent as any).code === '40001') return true + // 40P01 = deadlock detected + if ((parent as any).code === '40P01') return true + } else if (database.dialect === 'mariadb') { + const errno = (parent as any).errno + + // ER_LOCK_DEADLOCK + // Deadlock found when trying to get lock; try restarting transaction + if (errno === 1213) return true + } + + return false +} + +export async function assertSerializeableTransactionsAreWorking(database: Database) { + // clean up just for the case + await database.config.destroy({ + where: { + id: { + [Sequelize.Op.in]: [ configItemIds.selfTestData, configItemIds.secondSelfTestData ] + } + } + }) + + // insert specific data + await database.config.bulkCreate([ + { + id: configItemIds.selfTestData, + value: '123' + }, + { + id: configItemIds.secondSelfTestData, + value: '456' + } + ]) + + try { + // use two parallel transactions + await database.transaction(async (transactionOne) => { + await database.transaction(async (transactionTwo) => { + await database.config.findAll({ transaction: transactionOne }) + await database.config.findAll({ transaction: transactionTwo }) + + await Promise.all([ + (async () => { + await database.config.update({ value: 'c' }, { where: { id: configItemIds.selfTestData }, transaction: transactionOne }) + })(), + (async () => { + await database.config.update({ value: 'd' }, { where: { id: configItemIds.secondSelfTestData }, transaction: transactionTwo }) + })(), + ]) + }) + }) + + throw new SerializationFeatureCheckException() + } catch (ex) { + if (!shouldRetryWithException(database, ex)) { + throw new SerializationFeatureCheckException() + } + } + + // finish clean up + await database.config.destroy({ + where: { + id: { + [Sequelize.Op.in]: [ configItemIds.selfTestData, configItemIds.secondSelfTestData ] + } + } + }) +} diff --git a/src/function/sync/apply-actions/index.ts b/src/function/sync/apply-actions/index.ts index c83c902..e9b5ff8 100644 --- a/src/function/sync/apply-actions/index.ts +++ b/src/function/sync/apply-actions/index.ts @@ -18,7 +18,7 @@ import { BadRequest } from 'http-errors' import { ClientPushChangesRequest } from '../../../api/schema' import { VisibleConnectedDevicesManager } from '../../../connected-devices' -import { Database } from '../../../database' +import { Database, shouldRetryWithException } from '../../../database' import { EventHandler } from '../../../monitoring/eventhandler' import { WebsocketApi } from '../../../websocket' import { notifyClientsAboutChangesDelayed } from '../../websocket' @@ -104,7 +104,11 @@ export const applyActionsFromDevice = async ({ database, request, websocket, con } }) } catch (ex) { - if (ex instanceof ApplyActionException) { + if (shouldRetryWithException(ex)) { + eventHandler.countEvent('applyActionsFromDevice got exception which should cause retry') + + throw ex + } else if (ex instanceof ApplyActionException) { eventHandler.countEvent('applyActionsFromDevice errorDispatchingAction:' + ex.staticMessage) } else { const stack = ex instanceof Error && ex.stack ? ex.stack.substring(0, 4096) : 'no stack' diff --git a/src/index.ts b/src/index.ts index 260df4d..5d8530c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ /* * server component for the TimeLimit App - * Copyright (C) 2019 - 2020 Jonas Lochmann + * Copyright (C) 2019 - 2021 Jonas Lochmann * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -19,7 +19,7 @@ import { Server } from 'http' import { createApi } from './api' import { config } from './config' import { VisibleConnectedDevicesManager } from './connected-devices' -import { assertNestedTransactionsAreWorking, defaultDatabase, defaultUmzug } from './database' +import { assertNestedTransactionsAreWorking, assertSerializeableTransactionsAreWorking, defaultDatabase, defaultUmzug } from './database' import { EventHandler } from './monitoring/eventhandler' import { InMemoryEventHandler } from './monitoring/inmemoryeventhandler' import { createWebsocketHandler } from './websocket' @@ -31,6 +31,7 @@ async function main () { const eventHandler: EventHandler = new InMemoryEventHandler() await assertNestedTransactionsAreWorking(database) + await assertSerializeableTransactionsAreWorking(database) const connectedDevicesManager = new VisibleConnectedDevicesManager({ database