diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index e4c19b48c73981..46bcfa1de9c072 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -38,6 +38,8 @@ import { PrebuiltUpdatableAndWorkspace, WorkspaceAndOwner, WorkspaceDB, + WorkspaceOwnerAndContentDeletedTime, + WorkspaceOwnerAndDeletionEligibility, WorkspaceOwnerAndSoftDeleted, WorkspacePortsAuthData, } from "../workspace-db"; @@ -486,8 +488,7 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl imp cutOffDate: Date = new Date(), limit: number = 100, type: WorkspaceType = "regular", - ): Promise { - // we do not allow to run this with a future date + ): Promise { if (cutOffDate > new Date()) { throw new Error("cutOffDate must not be in the future, was: " + cutOffDate.toISOString()); } @@ -495,7 +496,8 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl imp const dbResults = await workspaceRepo.query( ` SELECT ws.id AS id, - ws.ownerId AS ownerId + ws.ownerId AS ownerId, + ws.deletionEligibilityTime AS deletionEligibilityTime FROM d_b_workspace AS ws WHERE ws.deleted = 0 AND ws.type = ? @@ -516,12 +518,12 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl imp minContentDeletionTimeInDays: number, limit: number, now: Date, - ): Promise { + ): Promise { const minPurgeTime = daysBefore(now.toISOString(), minContentDeletionTimeInDays); const repo = await this.getWorkspaceRepo(); const qb = repo .createQueryBuilder("ws") - .select(["ws.id", "ws.ownerId"]) + .select(["ws.id", "ws.ownerId", "ws.contentDeletedTime"]) .where(`ws.contentDeletedTime != ''`) .andWhere(`ws.contentDeletedTime < :minPurgeTime`, { minPurgeTime }) .andWhere(`ws.deleted = 0`) diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index 60669b3e0bcca2..eee3e1063960be 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -26,6 +26,7 @@ class WorkspaceDBSpec { readonly timeWs = new Date(2018, 2, 16, 10, 0, 0).toISOString(); readonly timeBefore = new Date(2018, 2, 16, 11, 5, 10).toISOString(); readonly timeAfter = new Date(2019, 2, 16, 11, 5, 10).toISOString(); + readonly timeAfter2 = new Date(2019, 2, 17, 4, 20, 10).toISOString(); readonly userId = "12345"; readonly projectAID = "projectA"; readonly projectBID = "projectB"; @@ -101,6 +102,30 @@ class WorkspaceDBSpec { deleted: false, usageAttributionId: undefined, }; + readonly wsi3: WorkspaceInstance = { + workspaceId: this.ws.id, + id: "12345", + ideUrl: "example.org", + region: "unknown", + workspaceClass: undefined, + workspaceImage: "abc.io/test/image:123", + creationTime: this.timeAfter2, + startedTime: undefined, + deployedTime: undefined, + stoppingTime: undefined, + stoppedTime: undefined, + status: { + version: 1, + phase: "stopped", + conditions: {}, + }, + configuration: { + theiaVersion: "unknown", + ideImage: "unknown", + }, + deleted: false, + usageAttributionId: undefined, + }; readonly ws2: Workspace = { id: "2", type: "regular", @@ -235,7 +260,7 @@ class WorkspaceDBSpec { } @test(timeout(10000)) - public async testFindEligableWorkspacesForSoftDeletion_markedEligable_Prebuild() { + public async testFindEligibleWorkspacesForSoftDeletion_markedEligible_Prebuild() { const { ws } = await this.createPrebuild(20, 15); const dbResult = await this.db.findEligibleWorkspacesForSoftDeletion(new Date(), 10, "prebuild"); expect(dbResult.length).to.equal(1); @@ -244,7 +269,7 @@ class WorkspaceDBSpec { } @test(timeout(10000)) - public async testFindEligableWorkspacesForSoftDeletion_notMarkedEligable_Prebuild() { + public async testFindEligibleWorkspacesForSoftDeletion_notMarkedEligible_Prebuild() { await this.createPrebuild(20, -7); const dbResult = await this.db.findEligibleWorkspacesForSoftDeletion(new Date(), 10, "prebuild"); expect(dbResult.length).to.eq(0); @@ -254,7 +279,7 @@ class WorkspaceDBSpec { public async testPrebuildGarbageCollection() { const { pbws } = await this.createPrebuild(20, 15); - // mimick the behavior of the Garbage Collector + // mimic the behavior of the Garbage Collector const gcWorkspaces = await this.db.findEligibleWorkspacesForSoftDeletion(new Date(), 10, "prebuild"); expect(gcWorkspaces.length).to.equal(1); @@ -311,17 +336,27 @@ class WorkspaceDBSpec { } @test(timeout(10000)) - public async testFindEligableWorkspacesForSoftDeletion_markedEligable() { + public async testFindEligibleWorkspacesForSoftDeletion_markedEligible() { this.ws.deletionEligibilityTime = this.timeWs; - await Promise.all([this.db.store(this.ws), this.db.storeInstance(this.wsi1), this.db.storeInstance(this.wsi2)]); + await Promise.all([ + this.db.store(this.ws), + this.db.storeInstance(this.wsi1), + this.db.storeInstance(this.wsi2), + this.db.storeInstance(this.wsi3), + ]); const dbResult = await this.db.findEligibleWorkspacesForSoftDeletion(new Date(this.timeAfter), 10); expect(dbResult[0].id).to.eq(this.ws.id); expect(dbResult[0].ownerId).to.eq(this.ws.ownerId); } @test(timeout(10000)) - public async testFindEligableWorkspacesForSoftDeletion_notMarkedEligable() { - await Promise.all([this.db.store(this.ws), this.db.storeInstance(this.wsi1), this.db.storeInstance(this.wsi2)]); + public async testFindEligibleWorkspacesForSoftDeletion_notMarkedEligible() { + await Promise.all([ + this.db.store(this.ws), + this.db.storeInstance(this.wsi1), + this.db.storeInstance(this.wsi2), + this.db.storeInstance(this.wsi3), + ]); const dbResult = await this.db.findEligibleWorkspacesForSoftDeletion(new Date(this.timeAfter), 10); expect(dbResult.length).to.eq(0); } @@ -768,6 +803,7 @@ class WorkspaceDBSpec { { id: "1", ownerId, + contentDeletedTime: d20180131, }, ]); } diff --git a/components/gitpod-db/src/workspace-db.ts b/components/gitpod-db/src/workspace-db.ts index 9336b193fe4888..e42a23186e2cdc 100644 --- a/components/gitpod-db/src/workspace-db.ts +++ b/components/gitpod-db/src/workspace-db.ts @@ -64,6 +64,8 @@ export interface PrebuildWithWorkspaceAndInstances { export type WorkspaceAndOwner = Pick; export type WorkspaceOwnerAndSoftDeleted = Pick; +export type WorkspaceOwnerAndDeletionEligibility = Pick; +export type WorkspaceOwnerAndContentDeletedTime = Pick; export const WorkspaceDB = Symbol("WorkspaceDB"); export interface WorkspaceDB { @@ -102,7 +104,7 @@ export interface WorkspaceDB { cutOffDate?: Date, limit?: number, type?: WorkspaceType, - ): Promise; + ): Promise; findWorkspacesForContentDeletion( minSoftDeletedTimeInDays: number, limit: number, @@ -111,7 +113,7 @@ export interface WorkspaceDB { minContentDeletionTimeInDays: number, limit: number, now: Date, - ): Promise; + ): Promise; findAllWorkspaces( offset: number, limit: number, diff --git a/components/server/src/config.ts b/components/server/src/config.ts index 33d4cf5651be47..c949a156ffba8d 100644 --- a/components/server/src/config.ts +++ b/components/server/src/config.ts @@ -84,10 +84,10 @@ export interface WorkspaceGarbageCollection { /** The minimal age of a workspace before it is marked as 'softDeleted' (= hidden for the user) */ minAgeDays: number; - /** The minimal age of a prebuild (incl. workspace) before it's content is deleted (+ marked as 'softDeleted') */ + /** The minimal age of a prebuild (incl. workspace) before its content is deleted (+ marked as 'softDeleted') */ minAgePrebuildDays: number; - /** The minimal number of days a workspace has to stay in 'softDeleted' before it's content is deleted */ + /** The minimal number of days a workspace has to stay in 'softDeleted' before its content is deleted */ contentRetentionPeriodDays: number; /** The maximum amount of workspaces whose content is deleted in one go */ diff --git a/components/server/src/jobs/workspace-gc.ts b/components/server/src/jobs/workspace-gc.ts index c3cc3f29f825cb..042ef7aba34dd3 100644 --- a/components/server/src/jobs/workspace-gc.ts +++ b/components/server/src/jobs/workspace-gc.ts @@ -29,7 +29,7 @@ import { StorageClient } from "../storage/storage-client"; * - find _any_ workspace "softDeleted" for long enough -> move to "contentDeleted" * - find _any_ workspace "contentDeleted" for long enough -> move to "purged" * - prebuilds are special in that: - * - the GC has a dedicated sub-task to move workspace of type "prebuid" from "to delete" (with a different threshold) -> to "contentDeleted" directly + * - the GC has a dedicated sub-task to move workspace of type "prebuild" from "to delete" (with a different threshold) -> to "contentDeleted" directly * - the "purging" takes care of all Prebuild-related sub-resources, too */ @injectable() @@ -55,6 +55,11 @@ export class WorkspaceGarbageCollector implements Job { return; } + log.info("workspace-gc: job started", { + workspaceMinAgeDays: this.config.workspaceGarbageCollection.minAgeDays, + prebuildMinAgeDays: this.config.workspaceGarbageCollection.minAgePrebuildDays, + }); + // Move eligible "regular" workspace -> softDeleted try { await this.softDeleteEligibleWorkspaces(); @@ -115,6 +120,10 @@ export class WorkspaceGarbageCollector implements Job { err, ); } + + log.info({ workspaceId: ws.id }, `workspace-gc: soft deleted a workspace`, { + deletionEligibilityTime: ws.deletionEligibilityTime, + }); } const afterDelete = new Date(); @@ -187,6 +196,9 @@ export class WorkspaceGarbageCollector implements Job { } catch (err) { log.error({ workspaceId: ws.id }, "workspace-gc: failed to purge workspace", err); } + log.info({ workspaceId: ws.id }, `workspace-gc: hard deleted a workspace`, { + contentDeletedTime: ws.contentDeletedTime, + }); } const afterDelete = new Date(); diff --git a/components/server/src/workspace/workspace-service.spec.db.ts b/components/server/src/workspace/workspace-service.spec.db.ts index 5e6293ef00eb60..e711f08d855765 100644 --- a/components/server/src/workspace/workspace-service.spec.db.ts +++ b/components/server/src/workspace/workspace-service.spec.db.ts @@ -700,7 +700,7 @@ describe("WorkspaceService", async () => { workspaceImage: "", }); - await svc.updateDeletionEligabilityTime(owner.id, ws.id); + await svc.updateDeletionEligibilityTime(owner.id, ws.id); const workspace = await svc.getWorkspace(owner.id, ws.id); expect(workspace).to.not.be.undefined; @@ -734,7 +734,7 @@ describe("WorkspaceService", async () => { workspaceImage: "", }); - await svc.updateDeletionEligabilityTime(owner.id, ws.id); + await svc.updateDeletionEligibilityTime(owner.id, ws.id); const workspace = await svc.getWorkspace(owner.id, ws.id); expect(workspace).to.not.be.undefined; @@ -770,7 +770,7 @@ describe("WorkspaceService", async () => { workspaceImage: "", }); - await svc.updateDeletionEligabilityTime(owner.id, ws.id); + await svc.updateDeletionEligibilityTime(owner.id, ws.id); const workspace = await svc.getWorkspace(owner.id, ws.id); expect(workspace).to.not.be.undefined; diff --git a/components/server/src/workspace/workspace-service.ts b/components/server/src/workspace/workspace-service.ts index 78f3e539d58db6..defd384e1f3c33 100644 --- a/components/server/src/workspace/workspace-service.ts +++ b/components/server/src/workspace/workspace-service.ts @@ -223,8 +223,8 @@ export class WorkspaceService { ); throw err; } - this.asyncUpdateDeletionEligabilityTime(user.id, workspace.id); - this.asyncUpdateDeletionEligabilityTimeForUsedPrebuild(user.id, workspace); + this.asyncUpdateDeletionEligibilityTime(user.id, workspace.id, true); + this.asyncUpdateDeletionEligibilityTimeForUsedPrebuild(user.id, workspace); if (project && workspace.type === "regular") { this.asyncHandleUpdatePrebuildTriggerStrategy({ ctx, project, workspace, user }); this.asyncStartPrebuild({ ctx, project, workspace, user }); @@ -375,7 +375,7 @@ export class WorkspaceService { return; } await this.workspaceStarter.stopWorkspaceInstance({}, instance.id, instance.region, reason, policy); - this.asyncUpdateDeletionEligabilityTime(userId, workspaceId); + this.asyncUpdateDeletionEligibilityTime(userId, workspaceId, true); } public async stopRunningWorkspacesForUser( @@ -396,13 +396,13 @@ export class WorkspaceService { reason, policy, ); - this.asyncUpdateDeletionEligabilityTime(userId, info.workspace.id); + this.asyncUpdateDeletionEligibilityTime(userId, info.workspace.id, false); }), ); return infos.map((instance) => instance.workspace); } - private asyncUpdateDeletionEligabilityTimeForUsedPrebuild(userId: string, workspace: Workspace): void { + private asyncUpdateDeletionEligibilityTimeForUsedPrebuild(userId: string, workspace: Workspace): void { (async () => { if (WithPrebuild.is(workspace.context) && workspace.context.prebuildWorkspaceId) { // mark the prebuild active @@ -410,7 +410,7 @@ export class WorkspaceService { workspace.context.prebuildWorkspaceId, ); if (prebuiltWorkspace?.buildWorkspaceId) { - await this.updateDeletionEligabilityTime(userId, prebuiltWorkspace?.buildWorkspaceId, true); + await this.updateDeletionEligibilityTime(userId, prebuiltWorkspace?.buildWorkspaceId, true); } } })().catch((err) => @@ -515,8 +515,8 @@ export class WorkspaceService { ); } - private asyncUpdateDeletionEligabilityTime(userId: string, workspaceId: string): void { - this.updateDeletionEligabilityTime(userId, workspaceId).catch((err) => + private asyncUpdateDeletionEligibilityTime(userId: string, workspaceId: string, activeNow?: boolean): void { + this.updateDeletionEligibilityTime(userId, workspaceId, activeNow).catch((err) => log.error({ userId, workspaceId }, "Failed to update deletion eligibility time", err), ); } @@ -524,26 +524,23 @@ export class WorkspaceService { /** * Sets the deletionEligibilityTime of the workspace, depending on the current state of the workspace and the configuration. * - * @param userId sets the - * @param workspaceId + * @param userId the user to act as + * @param workspaceId the workspace to update * @returns */ - async updateDeletionEligabilityTime(userId: string, workspaceId: string, activeNow = false): Promise { + async updateDeletionEligibilityTime(userId: string, workspaceId: string, activeNow = false): Promise { try { let daysToLive = this.config.workspaceGarbageCollection?.minAgeDays || 14; const daysToLiveForPrebuilds = this.config.workspaceGarbageCollection?.minAgePrebuildDays || 7; const workspace = await this.doGetWorkspace(userId, workspaceId); const instance = await this.db.findCurrentInstance(workspaceId); - let lastActive = + const lastActive = instance?.stoppingTime || instance?.startedTime || instance?.creationTime || workspace?.creationTime; - if (activeNow) { - lastActive = new Date().toISOString(); - } - if (!lastActive) { + if (!lastActive && !activeNow) { return; } - const deletionEligibilityTime = new Date(lastActive); + const deletionEligibilityTime = activeNow ? new Date() : new Date(lastActive); if (workspace.type === "prebuild") { // set to last active plus daysToLiveForPrebuilds as iso string deletionEligibilityTime.setDate(deletionEligibilityTime.getDate() + daysToLiveForPrebuilds); @@ -553,14 +550,39 @@ export class WorkspaceService { return; } // workspaces with pending changes live twice as long - if ( - (instance?.gitStatus?.totalUncommitedFiles || 0) > 0 || - (instance?.gitStatus?.totalUnpushedCommits || 0) > 0 || - (instance?.gitStatus?.totalUntrackedFiles || 0) > 0 - ) { + const hasGitChanges = + instance?.gitStatus?.totalUncommitedFiles || + 0 > 0 || + instance?.gitStatus?.totalUnpushedCommits || + 0 > 0 || + instance?.gitStatus?.totalUntrackedFiles || + 0 > 0; + if (hasGitChanges) { daysToLive = daysToLive * 2; } deletionEligibilityTime.setDate(deletionEligibilityTime.getDate() + daysToLive); + if ( + workspace.deletionEligibilityTime && + workspace.deletionEligibilityTime > deletionEligibilityTime.toISOString() + ) { + log.warn( + { userId, workspaceId, instanceId: instance?.id }, + "Prevented moving deletion eligibility time backwards", + { + hasGitChanges, + timestamps: new TrustedValue({ + wouldBeDeletionEligibilityTime: deletionEligibilityTime.toISOString(), + currentDeletionEligibilityTime: workspace.deletionEligibilityTime, + instanceStoppingTime: instance?.stoppingTime, + instanceStartedTime: instance?.startedTime, + instanceCreationTime: instance?.creationTime, + workspaceCreationTime: workspace.creationTime, + lastActive, + }), + }, + ); + return; + } await this.db.updatePartial(workspaceId, { deletionEligibilityTime: deletionEligibilityTime.toISOString(), }); @@ -819,7 +841,7 @@ export class WorkspaceService { // at this point we're about to actually start a new workspace const result = await this.workspaceStarter.startWorkspace(ctx, workspace, user, await projectPromise, options); - this.asyncUpdateDeletionEligabilityTime(user.id, workspaceId); + this.asyncUpdateDeletionEligibilityTime(user.id, workspaceId, true); return result; } @@ -928,7 +950,7 @@ export class WorkspaceService { const workspace = await this.doGetWorkspace(userId, workspaceId); instance = await this.db.updateInstancePartial(instance.id, { gitStatus }); - await this.updateDeletionEligabilityTime(userId, workspaceId); + await this.updateDeletionEligibilityTime(userId, workspaceId, true); await this.publisher.publishInstanceUpdate({ instanceID: instance.id, ownerID: workspace.ownerId, diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index 5d41e278e28a86..921ac2708fba7a 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -490,7 +490,7 @@ export class WorkspaceStarter { client = await this.clientProvider.get(instanceRegion); } catch (err) { log.error({ instanceId }, "cannot stop workspace instance", err); - // we want to stop a workspace but the region doesn't exist. So we can assume it doesn't run anyymore and there will never be updates coming to bridge. + // we want to stop a workspace but the region doesn't exist. So we can assume it doesn't run anymore and there will never be updates coming to bridge. // let's mark this workspace as stopped if it is not already stopped. const workspace = await this.workspaceDb.trace(ctx).findByInstanceId(instanceId); const instance = await this.workspaceDb.trace(ctx).findInstanceById(instanceId);