[ws-gc] WS soft deletion improvements #20271
Conversation
filiptronicek
changed the title
| // we do not allow to run this with a future date | ||
| if (cutOffDate > new Date()) { | ||
| throw new Error("cutOffDate must not be in the future, was: " + cutOffDate.toISOString()); | ||
| } | ||
| const workspaceRepo = await this.getWorkspaceRepo(); | ||
| const dbResults = await workspaceRepo.query( |
There was a problem hiding this comment.
I wanted to align this with findWorkspacesForPurging by converting from a raw SQL statement, but maybe it's too out of scope here. I'm curious if anyone has thoughts about it
There was a problem hiding this comment.
Looking at the "async predicate" which looks way too much like a join in disguise, let' snot do this right now. 👍 🙃
There was a problem hiding this comment.
WDYT about the readability of this with 13e1a76 (#20271)? I tried putting everything into a single query and think that it makes a bit more sense.
The benefit of using this join is not too big though so I'm happy to revert if you think it's not that helpful.
| this.db.store(this.ws), | ||
| this.db.storeInstance(this.wsi1), | ||
| this.db.storeInstance(this.wsi2), | ||
| this.db.storeInstance(this.wsi3), |
| @@ -561,6 +561,12 @@ export class WorkspaceService { | |||
| daysToLive = daysToLive * 2; | |||
| } | |||
| deletionEligibilityTime.setDate(deletionEligibilityTime.getDate() + daysToLive); | |||
| if (new Date() > deletionEligibilityTime) { | |||
| log.warn({ userId, workspaceId }, "Prevented setting deletion eligibility time in the past", { | |||
There was a problem hiding this comment.
🫧 I wonder whether we are also interested in the "reason" for the update. But I guess we can see that from looking at the context in the logs later on. 👍
There was a problem hiding this comment.
Added more details in 39065c7 (#20271), WDYT?
|
Here's a sample log event when adding git changes and then discarding them: {
"@type": "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent",
"serviceContext": {
"service": "server",
"version": "ft-additioda303f614e-dev-2024-10-08_T18-33-52"
},
"component": "server",
"severity": "WARNING",
"time": "2024-10-08T18:55:11.397Z",
"context": {
"requestId": "bf218162-f8d9-4fb2-aaab-095129de31ed",
"requestKind": "jsonrpc",
"requestMethod": "updateGitStatus",
"traceId": "",
"subjectId": "user_dd98d787-76ed-4114-abee-9acbdda79786",
"userId": "dd98d787-76ed-4114-abee-9acbdda79786",
"contextTimeMs": 27.451227009296417,
"workspaceId": "[redacted:md5:0e98a0a31bc6576effd2fcd40f668b24]"
},
"message": "Prevented moving deletion eligibility time backwards",
"payload": {
"hasGitChanges": false,
"timestamps": {
"wouldBeDeletionEligibilityTime": "2024-10-22T18:55:11.397Z",
"currentDeletionEligibilityTime": "2024-11-05T18:53:26.402Z",
"instanceStartedTime": "2024-10-08T18:52:32.735Z",
"instanceCreationTime": "2024-10-08T18:52:20.175Z",
"workspaceCreationtime": "2024-10-08T18:52:20.112Z",
"lastActive": "2024-10-08T18:52:32.735Z"
}
}
}cc @geropl |
There was a problem hiding this comment.
Code LGTM, thanks for jumping through the hoops together @filiptronicek ! 🧡
Description
Makes some improvements on our workspace garbage collection we periodically run:
workspaceMinAgeDaysandprebuildMinAgeDayswhen the job starts, as well as logging every soft- and hard-deleted entry along with its workspace ID anddeletionEligibilityTime.deletionEligibilityTimeof a workspace from looking at the latest instance'sstoppingTime,startedTimeorcreationTimeor workspace'screationTimeto actually using the current time if we're calling the update as part of a stop, start, create or git status update.Related Issue(s)
Fixes ENT-854
How to test
See test updates