From 58476d50474199886cdb8420f116fa10ff6d5a61 Mon Sep 17 00:00:00 2001 From: Mahmoud Bahaa Date: Fri, 13 Oct 2023 19:45:10 +0300 Subject: [PATCH] add clear db that clear thd b at start which is only set in true in test server and a soft delete option that can be used to debug gc pproblems --- src/blob/SqlBlobConfiguration.ts | 8 +- src/blob/SqlBlobServer.ts | 7 +- src/blob/persistence/SqlBlobMetadataStore.ts | 217 ++++++++---------- src/common/ConfigurationBase.ts | 2 + .../persistence/SqlExtentMetadataStore.ts | 11 +- tests/BlobTestServerFactory.ts | 6 +- 6 files changed, 116 insertions(+), 135 deletions(-) diff --git a/src/blob/SqlBlobConfiguration.ts b/src/blob/SqlBlobConfiguration.ts index a6e8b0586..12b34d473 100644 --- a/src/blob/SqlBlobConfiguration.ts +++ b/src/blob/SqlBlobConfiguration.ts @@ -35,7 +35,9 @@ export default class SqlBlobConfiguration extends ConfigurationBase { key: string = "", pwd: string = "", oauth?: string, - disableProductStyleUrl: boolean = false + disableProductStyleUrl: boolean = false, + clearDB: boolean = false, + softDelete: boolean = true ) { super( host, @@ -50,7 +52,9 @@ export default class SqlBlobConfiguration extends ConfigurationBase { key, pwd, oauth, - disableProductStyleUrl + disableProductStyleUrl, + clearDB, + softDelete ); } } diff --git a/src/blob/SqlBlobServer.ts b/src/blob/SqlBlobServer.ts index c0e07e6d3..868c05a79 100644 --- a/src/blob/SqlBlobServer.ts +++ b/src/blob/SqlBlobServer.ts @@ -66,14 +66,17 @@ export default class SqlBlobServer extends ServerBase { const metadataStore: IBlobMetadataStore = new SqlBlobMetadataStore( configuration.sqlURL, - configuration.sequelizeOptions + configuration.sequelizeOptions, + configuration.clearDB, + configuration.softDelete ); const extentMetadataStore: IExtentMetadataStore = new SqlExtentMetadataStore( // Currently, extent metadata and blob metadata share same database // But they can use separate databases per future requirements configuration.sqlURL, - configuration.sequelizeOptions + configuration.sequelizeOptions, + configuration.clearDB ); const extentStore: IExtentStore = new FSExtentStore( diff --git a/src/blob/persistence/SqlBlobMetadataStore.ts b/src/blob/persistence/SqlBlobMetadataStore.ts index ad0d96264..8e1e3b0c2 100644 --- a/src/blob/persistence/SqlBlobMetadataStore.ts +++ b/src/blob/persistence/SqlBlobMetadataStore.ts @@ -108,7 +108,9 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { */ public constructor( connectionURI: string, - sequelizeOptions?: SequelizeOptions + sequelizeOptions?: SequelizeOptions, + private readonly clearDB: boolean = false, + private readonly softDelete: boolean = true ) { // Enable encrypt connection for SQL Server if (connectionURI.startsWith("mssql") && sequelizeOptions) { @@ -365,8 +367,12 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { } ); - // TODO: sync() is only for development purpose, use migration for production - await this.sequelize.sync(); + if (this.clearDB) { + await this.sequelize.sync({ force: true }); + } else { + // TODO: sync() is only for development purpose, use migration for production + await this.sequelize.sync(); + } this.initialized = true; } @@ -642,33 +648,8 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { transaction: t }); - // TODO: GC blobs under deleting status - await BlobsModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container - }, - transaction: t - } - ); - - // TODO: GC blocks under deleting status - await BlocksModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container - }, - transaction: t - } - ); + await this.destroyBlob(account, container, undefined, undefined, t); + await this.destroyBlock(account, container, undefined, undefined, t); /* Transaction ends */ }); } @@ -1439,7 +1420,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { }); } - public getBlockList( + public async getBlockList( context: Context, account: string, container: string, @@ -1667,18 +1648,12 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { transaction: t }); - await BlocksModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: blob.accountName, - containerName: blob.containerName, - blobName: blob.name - }, - transaction: t - } + await this.destroyBlock( + blob.accountName, + blob.containerName, + blob.name, + undefined, + t ); }); } @@ -1891,52 +1866,14 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { if (count > 1) { throw StorageErrorFactory.getSnapshotsPresent(context.contextId!); } else { - await BlobsModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob - }, - transaction: t - } - ); - - await BlocksModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob - }, - transaction: t - } - ); + await this.destroyBlob(account, container, blob, undefined, t); + await this.destroyBlock(account, container, blob, undefined, t); } } // Scenario: Delete one snapshot only if (!againstBaseBlob) { - await BlobsModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob, - snapshot: blobModel.snapshot - }, - transaction: t - } - ); + await this.destroyBlob(account, container, blob, blobModel.snapshot, t); } // Scenario: Delete base blob and snapshots @@ -1944,33 +1881,8 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { againstBaseBlob && options.deleteSnapshots === Models.DeleteSnapshotsOptionType.Include ) { - await BlobsModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob - }, - transaction: t - } - ); - - await BlocksModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob - }, - transaction: t - } - ); + await this.destroyBlob(account, container, blob, undefined, t); + await this.destroyBlock(account, container, blob, undefined, t); } // Scenario: Delete all snapshots only @@ -1978,20 +1890,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { againstBaseBlob && options.deleteSnapshots === Models.DeleteSnapshotsOptionType.Only ) { - await BlobsModel.update( - { - deleting: literal("deleting + 1") - }, - { - where: { - accountName: account, - containerName: container, - blobName: blob, - snapshot: { [Op.gt]: "" } - }, - transaction: t - } - ); + await this.destroyBlob(account, container, blob, { [Op.gt]: "" }, t); } }); } @@ -2072,7 +1971,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { }); } - public setBlobMetadata( + public async setBlobMetadata( context: Context, account: string, container: string, @@ -3447,4 +3346,70 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { } return undefined; } + + private async destroyBlob( + account: string, + container: string, + blob: string | undefined, + snapshot: any, + t: Transaction + ): Promise { + const where: any = { + accountName: account, + containerName: container + }; + + if (blob !== undefined) where.blobName = blob; + if (snapshot !== undefined) where.snapshot = snapshot; + + if (this.softDelete) { + await BlobsModel.update( + { + deleting: literal("deleting + 1") + }, + { + where, + transaction: t + } + ); + } else { + await BlobsModel.destroy({ + where, + transaction: t + }); + } + } + + private async destroyBlock( + account: string, + container: string, + blob: string | undefined, + snapshot: any, + t: Transaction + ): Promise { + const where: any = { + accountName: account, + containerName: container + }; + + if (blob !== undefined) where.blobName = blob; + if (snapshot !== undefined) where.snapshot = snapshot; + + if (this.softDelete) { + await BlocksModel.update( + { + deleting: literal("deleting + 1") + }, + { + where, + transaction: t + } + ); + } else { + await BlocksModel.destroy({ + where, + transaction: t + }); + } + } } diff --git a/src/common/ConfigurationBase.ts b/src/common/ConfigurationBase.ts index 42336eec7..f43fd1d04 100644 --- a/src/common/ConfigurationBase.ts +++ b/src/common/ConfigurationBase.ts @@ -22,6 +22,8 @@ export default abstract class ConfigurationBase { public readonly pwd: string = "", public readonly oauth?: string, public readonly disableProductStyleUrl: boolean = false, + public readonly clearDB: boolean = false, + public readonly softDelete: boolean = true ) {} public hasCert() { diff --git a/src/common/persistence/SqlExtentMetadataStore.ts b/src/common/persistence/SqlExtentMetadataStore.ts index f8ffb6a28..928e32c48 100644 --- a/src/common/persistence/SqlExtentMetadataStore.ts +++ b/src/common/persistence/SqlExtentMetadataStore.ts @@ -34,7 +34,8 @@ export default class SqlExtentMetadataStore implements IExtentMetadataStore { */ public constructor( connectionURI: string, - sequelizeOptions?: SequelizeOptions + sequelizeOptions?: SequelizeOptions, + private readonly clearDB: boolean = false, ) { // Enable encrypt connection for SQL Server if (connectionURI.startsWith("mssql") && sequelizeOptions) { @@ -74,8 +75,12 @@ export default class SqlExtentMetadataStore implements IExtentMetadataStore { { sequelize: this.sequelize, modelName: "Extents", timestamps: false } ); - // TODO: Remove this part which only for test. - await this.sequelize.sync(); + if (this.clearDB) { + await this.sequelize.sync({ force: true }); + } else { + // TODO: Remove this part which only for test. + await this.sequelize.sync(); + } this.initialized = true; } diff --git a/tests/BlobTestServerFactory.ts b/tests/BlobTestServerFactory.ts index c37c19848..d4f0c6ebb 100644 --- a/tests/BlobTestServerFactory.ts +++ b/tests/BlobTestServerFactory.ts @@ -43,7 +43,8 @@ export default class BlobTestServerFactory { cert, key, undefined, - oauth + oauth, + true ); return new SqlBlobServer(config); @@ -65,7 +66,8 @@ export default class BlobTestServerFactory { cert, key, undefined, - oauth + oauth, + true ); return new BlobServer(config); }