Repository navigation
Improve delete of workspaces - #4047
Conversation
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt> On-behalf-of: @SAP mangirdas.judeikis@sap.com
kcp-ci-bot
added
release-note
|
Skipping CI for Draft Pull Request. |
kcp-ci-bot
added
kind/feature
|
/test all |
kcp-ci-bot
removed
the
do-not-merge/work-in-progress
| if !conditions.IsTrue(workspace, tenancyv1alpha1.WorkspaceContentDeleted) { | ||
| after := time.Since(logicalCluster.CreationTimestamp.Time) / 5 | ||
| if max := time.Minute * 10; after > max { | ||
| if max := time.Minute * 5; after > max { |
There was a problem hiding this comment.
Why are we maxing out at five minutes? I feel like we should put in a comment why we chose this over the previous 10m.
There was a problem hiding this comment.
I'm just not a huge fan of undocumented magic numbers :D
There was a problem hiding this comment.
I don't know the original number, either. It's just some arbitrary time "for workspace delete" to succeed.
10 min per test was WAY too long.
TBH not sure what even todo here :D Not enough data at scale to make a very informed decision.
There was a problem hiding this comment.
added explanation :)
ntnn
left a comment
ntnn
left a comment
There was a problem hiding this comment.
Otherwise looks good on a quick look.
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ntnn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: 963c4f0022ef8b07f97d59ecec595151e2d82d8c |
Summary
Currently, workspace deletion is based on pooling.
This will wire informer-based deletes and index for logicalClusters.
In addition, reduce pooling to 5 mins from the previous 10.
This should improve the speed at which workspaces are deleted when logicalcluster goes away.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #4012
Release Notes