From 703890dc541c180e79a1b17987658ce1d8246d19 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 28 Sep 2022 06:55:20 +0000 Subject: [PATCH 1/5] Soft delete volume snapshot db entries Replace the existing hard-deletion of volume snapshot rows in the database with soft-deletion that will allow `db-sync` to propagate deletions between clusters. --- components/gitpod-db/src/typeorm/entity/db-volume-snapshot.ts | 4 ++++ components/gitpod-db/src/typeorm/workspace-db-impl.ts | 2 +- components/gitpod-db/src/workspace-db.spec.db.ts | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/components/gitpod-db/src/typeorm/entity/db-volume-snapshot.ts b/components/gitpod-db/src/typeorm/entity/db-volume-snapshot.ts index 079006d3caec33..b9e5518a593b0c 100644 --- a/components/gitpod-db/src/typeorm/entity/db-volume-snapshot.ts +++ b/components/gitpod-db/src/typeorm/entity/db-volume-snapshot.ts @@ -31,4 +31,8 @@ export class DBVolumeSnapshot implements VolumeSnapshot { @Column("varchar") volumeHandle: string; + + // This column triggers the db-sync deletion mechanism. It's not intended for public consumption. + @Column() + deleted: boolean; } diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index 88fcc39f907dee..8e6872f207e573 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -775,7 +775,7 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { public async deleteVolumeSnapshot(volumeSnapshotId: string): Promise { const volumeSnapshots = await this.getVolumeSnapshotRepo(); - await volumeSnapshots.delete(volumeSnapshotId); + await volumeSnapshots.update(volumeSnapshotId, { deleted: true }); } public async updateVolumeSnapshot( diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index ae24c1a83e4d34..15898a3b54302b 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -682,12 +682,14 @@ class WorkspaceDBSpec { expect(vss).to.deep.equal([ { creationTime: "", + deleted: false, id: "456", volumeHandle: "some-handle2", workspaceId: "ws-123", }, { creationTime: "", + deleted: false, id: "123", volumeHandle: "some-handle", workspaceId: "ws-123", From 962b64e774577f1e92f1897ea6d8f532c4605561 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 29 Sep 2022 06:58:28 +0000 Subject: [PATCH 2/5] Red: Add deleted vol snapshots to the test setup For each workspaceId, add a deleted volume snapshot. --- .../gitpod-db/src/workspace-db.spec.db.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index 15898a3b54302b..4bfc2522322eda 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -719,6 +719,16 @@ class WorkspaceDBSpec { workspaceId, }); + const longBeforeNow = new Date(2018, 2, 15, 10, 0, 0).toISOString(); + const id2b = "456b"; + await repo.save({ + id: id2b, + creationTime: longBeforeNow, + volumeHandle: "some-handle2b", + workspaceId, + deleted: true, + }); + const someOtherTime = new Date(2018, 2, 10, 6, 5).toISOString(); const id3 = "789"; const workspaceId2 = "ws-789"; @@ -728,6 +738,16 @@ class WorkspaceDBSpec { volumeHandle: "some-handle3", workspaceId: workspaceId2, }); + + const yetAnotherTime = new Date(2018, 2, 10, 5, 5).toISOString(); + const id4 = "012"; + await repo.save({ + id: id4, + creationTime: yetAnotherTime, + volumeHandle: "some-handle4", + workspaceId: workspaceId2, + deleted: true, + }); } @test(timeout(10000)) From ff63c363a968415d67252c43ed9893906dc2a5e6 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 29 Sep 2022 06:59:46 +0000 Subject: [PATCH 3/5] Green: Don't consider deleted volume snapshots Filter out deleted volume snapshots from the result sets. --- components/gitpod-db/src/typeorm/workspace-db-impl.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index 8e6872f207e573..6837eede7717c6 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -792,6 +792,7 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { const qb = volumeSnapshotRepo .createQueryBuilder("vs") .select("vs.workspaceId", "workspaceId") + .where("vs.deleted = 0") .groupBy("vs.workspaceId") .having("COUNT(*) > 1") .limit(limit); @@ -805,6 +806,7 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { const results = await volumeSnapshotRepo .createQueryBuilder("vs") .where("vs.workspaceId = :wsId", { wsId }) + .andWhere("vs.deleted = 0") .orderBy("vs.creationTime", "DESC") .limit(limit) .getMany(); From 25916c7ffae265e1eb1f6670561cbe88799f3acb Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 29 Sep 2022 07:09:22 +0000 Subject: [PATCH 4/5] Red: Add tests for `findVolumeSnapshotById` Add tests (`testCantFindDeletedVolumeSnapshotById` fails). The failing test is due to `findVolumeSnapshotById` not taking deleted entries into account. --- .../gitpod-db/src/workspace-db.spec.db.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index 4bfc2522322eda..72d709aef9dd62 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -663,6 +663,24 @@ class WorkspaceDBSpec { } } + @test(timeout(10000)) + public async testCanFindVolumeSnapshotById() { + await this.threeVolumeSnapshotsForTwoWorkspaces(); + + const volSnapshot = await this.db.findVolumeSnapshotById("123"); + + expect(volSnapshot?.id).to.eq("123"); + } + + @test(timeout(10000)) + public async testCantFindDeletedVolumeSnapshotById() { + await this.threeVolumeSnapshotsForTwoWorkspaces(); + + const volSnapshot = await this.db.findVolumeSnapshotById("456b"); + + expect(volSnapshot).to.be.undefined; + } + @test(timeout(10000)) public async testFindVolumeSnapshotWorkspacesForGC() { await this.threeVolumeSnapshotsForTwoWorkspaces(); From 3f1dc6619f530c57e4ba6898f468421f5f357a2d Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 29 Sep 2022 09:08:23 +0000 Subject: [PATCH 5/5] Green: Make `findVolumeSnapshotById` delete-aware Filter soft deleted entries out of the result set. --- components/gitpod-db/src/typeorm/workspace-db-impl.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index 6837eede7717c6..451e46d0a30b6a 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -764,7 +764,11 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { public async findVolumeSnapshotById(volumeSnapshotId: string): Promise { const volumeSnapshots = await this.getVolumeSnapshotRepo(); - return volumeSnapshots.findOne(volumeSnapshotId); + return volumeSnapshots + .createQueryBuilder("vs") + .where("vs.deleted = 0") + .andWhere("vs.id = :id", { id: volumeSnapshotId }) + .getOne(); } public async storeVolumeSnapshot(volumeSnapshot: VolumeSnapshot): Promise {