[ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler #2552

geropl · 2020-12-16T07:55:01Z

This:

enables asynchronous binding of pod to node
moves the duty of deleting a ghost workspace for each ~~REGULAR~~ non-GHOST workspace start from ws-manager to ws-scheduler
hides ghost workspaces from scheduler strategies: this relies on the fact that we do 2. ☝️

TBD:

add tests
load test

geropl · 2020-12-16T07:58:54Z

/werft run

👍 started the job as gitpod-build-gpl-2513-scheduler-deletes-ghosts.3

geropl · 2020-12-16T08:05:36Z

/werft run

👍 started the job as gitpod-build-gpl-2513-scheduler-deletes-ghosts.4

🤞

components/ee/ws-scheduler/pkg/scheduler/scheduler.go

geropl · 2020-12-16T12:21:14Z

/werft run

👍 started the job as gitpod-build-gpl-2513-scheduler-deletes-ghosts.6

geropl · 2020-12-16T13:23:20Z

I just performed a load test, which sadly triggered some OutOfMemory errors again. Will revise the code again, but not before tomorrow.

geropl · 2021-01-06T13:23:54Z

Actually I think the current issue is an instance of what be discussed above: In rare cases the 1 second is not enough and kubelet and ws-scheduler are out-of-sync, resulting in out-of-memory issues.

Next attempt would be to increase the (max) grace period to maximum (30s AFAIR).

Positively speaking: During the load test only in ~4% of the cases the delete took longer than 1s! 🙃

geropl · 2021-01-13T16:22:18Z

Set deletionGracePeriod to 30s. That worked well for a 100 ws test on staging. 🎉

geropl · 2021-01-14T11:30:04Z

Just coming back and think maybe it's better to use a ctx.WithTimeout to ghost.Delete (new api available since k8s API bump) to make sure we never run over the gracePeriod. This would have the effect that we skip this round of scheduling, but at most for scanning interval (2s currently). I think that would be more robust and also more responsive. Assuming pod.Delete(ctx, ...) handles cancel grateful, that is.

@csweichel

geropl · 2021-01-20T09:47:19Z

@csweichel ping

components/ee/ws-scheduler/pkg/scheduler/scheduler.go

- remove a ghost before binding a regular workspace - make ghosts "invisible" to strategy

components/ee/ws-scheduler/pkg/scheduler/scheduler.go

geropl · 2021-01-27T13:47:48Z

Another load test with 200 workspaces, beec427 and 4297176 went well.

components/ee/ws-scheduler/pkg/scheduler/common.go

components/ee/ws-scheduler/pkg/scheduler/scheduler.go

csweichel · 2021-01-27T15:13:04Z

components/ee/ws-scheduler/pkg/scheduler/state.go

 	for _, n := range nodes {
 		nds[n.Name] = &Node{
 			Node:     n,
 			Services: make(map[string]struct{}),
 		}
-		nodeToPod[n.Name] = make(map[string]struct{})
+		nodeToPod[n.Name] = &ntp{


why not make this part of nds?

Good question - I left it as I found it. 🙃
But the reasons seems to be that the book keeping is done during the process of adding pods from pds to each node in nds here. So it make sense to keep them separate.

csweichel · 2021-01-27T15:17:16Z

Didn't dive into the logic of things, hence the vanity comments

csweichel

LGTM pending the offline discussed changes and squash

…em with scheduler (gitpod-io#2552) * [ws-manager] Do not delete ghost workspace on start * [ws-scheduler] Enable asynchronous binding of pods * [ws-scheduler] Introduce ghosts - remove a ghost before binding a regular workspace - make ghosts "invisible" to strategy * [scheduler] Wait longer on ghost deletion to prevent OOM errors * [scheduler] Make isRegularWorkspace -> makeGhostsInvisible explicit * [scheduler] cancel ghost.Delete if it takes too long (5s) * [ws-scheduler] Add tests for ghost-sepcific state computation * [scheduler] Make sure ghost are only selected for deletion once * [scheduler] delete ghosts: ctxDeleteTimeout > gracePeriod * [scheduler] Don't bind terminated pods * [scheduler] Make all non-ghost workspaces replace ghosts * [scheduler] review comments

geropl commented Dec 16, 2020

View reviewed changes

components/ee/ws-scheduler/pkg/scheduler/scheduler.go Show resolved Hide resolved

geropl commented Dec 16, 2020

View reviewed changes

components/ee/ws-scheduler/pkg/scheduler/scheduler.go Outdated Show resolved Hide resolved

csweichel reviewed Dec 16, 2020

View reviewed changes

components/ee/ws-scheduler/pkg/scheduler/scheduler.go Outdated Show resolved Hide resolved

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch from 7133da8 to 130b2e0 Compare December 16, 2020 12:09

csweichel added this to the January 2021 milestone Jan 5, 2021

geropl added the component: ws-scheduler label Jan 6, 2021

csweichel mentioned this pull request Jan 8, 2021

[ws-scheduler] Delete ghost workspaces during scheduling, not workspace start #2513

Closed

csweichel removed this from the January 2021 milestone Jan 8, 2021

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch from 130b2e0 to e9b1857 Compare January 13, 2021 15:24

geropl marked this pull request as ready for review January 14, 2021 07:40

geropl changed the title ~~[ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler~~ [wip][ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler Jan 14, 2021

geropl changed the title ~~[wip][ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler~~ [ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler Jan 14, 2021

geropl requested a review from csweichel January 14, 2021 07:42

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch from cf0daa9 to 5a04770 Compare January 20, 2021 09:43

geropl commented Jan 21, 2021

View reviewed changes

components/ee/ws-scheduler/pkg/scheduler/scheduler.go Outdated Show resolved Hide resolved

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch from 5a04770 to 7d162a1 Compare January 26, 2021 09:10

geropl added 6 commits January 26, 2021 09:40

[ws-manager] Do not delete ghost workspace on start

4ed54e0

[ws-scheduler] Enable asynchronous binding of pods

96c2ca8

[ws-scheduler] Introduce ghosts

7b864ba

- remove a ghost before binding a regular workspace - make ghosts "invisible" to strategy

[scheduler] Wait longer on ghost deletion to prevent OOM errors

7a18d3d

[scheduler] Make isRegularWorkspace -> makeGhostsInvisible explicit

8aa2262

[scheduler] cancel ghost.Delete if it takes too long (5s)

446b6ce

geropl added 5 commits January 26, 2021 09:40

[ws-scheduler] Add tests for ghost-sepcific state computation

1e4a50f

[scheduler] Make sure ghost are only selected for deletion once

beec427

[scheduler] delete ghosts: ctxDeleteTimeout > gracePeriod

4297176

[scheduler] Don't bind terminated pods

f26a0df

[scheduler] Make all non-ghost workspaces replace ghosts

9b7a894

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch 2 times, most recently from 35f1119 to f13f335 Compare January 27, 2021 13:35

geropl commented Jan 27, 2021

View reviewed changes

components/ee/ws-scheduler/pkg/scheduler/scheduler.go Show resolved Hide resolved

csweichel reviewed Jan 27, 2021

View reviewed changes

csweichel approved these changes Jan 27, 2021

View reviewed changes

[scheduler] review comments

5553fc1

geropl force-pushed the gpl/2513-scheduler-deletes-ghosts branch from f13f335 to 5553fc1 Compare January 27, 2021 15:53

geropl merged commit 54b314b into master Jan 27, 2021

geropl deleted the gpl/2513-scheduler-deletes-ghosts branch January 27, 2021 16:07

[ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler #2552

[ws-scheduler] Make ghost workspaces more effective by integrating them with scheduler #2552

Uh oh!

Conversation

geropl commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented Dec 16, 2020 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented Dec 16, 2020 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

geropl commented Dec 16, 2020 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented Dec 16, 2020

Uh oh!

geropl commented Jan 6, 2021

Uh oh!

geropl commented Jan 13, 2021

Uh oh!

geropl commented Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented Jan 20, 2021

Uh oh!

Uh oh!

Uh oh!

geropl commented Jan 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csweichel Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

geropl Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

csweichel commented Jan 27, 2021

Uh oh!

csweichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

geropl commented Dec 16, 2020 •

edited

Loading

geropl commented Dec 16, 2020 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Dec 16, 2020 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Dec 16, 2020 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Jan 14, 2021 •

edited

Loading