[server] incremental workspaces (1 of 2) #14303

svenefftinge · 2022-10-31T22:07:41Z

Attention

Please review and approve #14461 which includes these changes

Description

This change adds a project preference that allows to generally use previous prebuilds and incrementally update the workspace when creating a workspace:

Related Issue(s)

See #12582

How to test

create a team and a project
enable the project setting
start a prebuild on that project
start a workspace while a prebuild is running and see that you get a workspace starting from git.
wait for the prebuild to finish
add a commit on top so that a prebuild is running again
start a workspace while a prebuild is running and see that you get a workspace starting from git.
change the new preference in project settings
start a workspace while the prebuild is running and see that you get a workspace based on the previous prebuild.

Release Notes

Introduces new project setting to use old prebuilds on workspace start

Documentation

Werft options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide

werft-gitpod-dev-com · 2022-10-31T22:07:49Z

started the job as gitpod-build-se-inc-ws.1 because the annotations in the pull request description changed
(with .werft/ from main)

components/dashboard/src/projects/Project.tsx

svenefftinge · 2022-10-31T23:55:16Z

components/dashboard/src/settings/selectClass.tsx

@@ -57,7 +57,6 @@ export default function SelectWorkspaceClass(props: SelectWorkspaceClassProps) {
    } else {
        return (
            <div>
-                <h3 className="mt-12">Workspaces</h3>


Moved to the call site

svenefftinge · 2022-10-31T23:57:26Z

components/dashboard/src/start/CreateWorkspace.tsx

+            getGitpodService()
+                .server.getWorkspace(workspaceId)
+                .then((ws) => {
+                    if (ws.latestInstance?.status?.phase === "stopped") {


polling here every 2 secs is much simpler (and actually works). Listening to events has a problem where we could have missed the 'stopped' event and then hang forever

Love this! I'm a huge fan of this simpler approach, and it also aligns much better with where we are looking to go with Pub API.

I'm fine with either way. Can we use repeat or any other mechnaims that ensures we cannot pile up requests here? ☁️

svenefftinge · 2022-10-31T23:59:00Z

components/server/ee/src/workspace/gitpod-server-impl.ts

@@ -1065,70 +1067,8 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
                    return;
                }



everything deleted below is no longer needed because we are now polling for prebuild finish. See above ☝️

easyCZ

LGTM. I would recommend splitting up the polling changes from the rest of the incremental workspaces feature.

/hold if you'd like a review from someone else also, I may not have 100% of the context here

geropl · 2022-11-04T07:58:01Z

Starting to review now.

geropl · 2022-11-04T08:04:05Z

components/dashboard/src/start/CreateWorkspace.tsx

@@ -262,7 +265,9 @@ export default class CreateWorkspace extends React.Component<CreateWorkspaceProp
                        </>
                    </div>
                    <div className="flex justify-end mt-6">
-                        <button onClick={() => this.createWorkspace(CreateWorkspaceMode.Default)}>New Workspace</button>
+                        <button onClick={() => this.createWorkspace({ ignoreRunningWorkspaceOnSameCommit: true })}>


geropl · 2022-11-04T08:04:24Z

components/dashboard/src/start/CreateWorkspace.tsx

@@ -148,7 +151,7 @@ export default class CreateWorkspace extends React.Component<CreateWorkspaceProp
                            <button
                                className=""
                                onClick={() => {
-                                    this.createWorkspace(CreateWorkspaceMode.Default, true);
+                                    this.createWorkspace({ forceDefaultConfig: true });


geropl · 2022-11-04T08:11:35Z

components/gitpod-protocol/src/gitpod-service.ts

@@ -421,7 +420,12 @@ export namespace GitpodServer {
    }
    export interface CreateWorkspaceOptions {
        contextUrl: string;
-        mode?: CreateWorkspaceMode;
+        // whether running prebuilds should be ignored.


geropl · 2022-11-04T08:13:29Z

components/gitpod-protocol/src/protocol.ts

@@ -211,6 +211,12 @@ export interface AdditionalUserData {
    dotfileRepo?: string;
    // preferred workspace classes
    workspaceClasses?: WorkspaceClasses;
+    // whether running prebuilds should be ignored on start
+    ignoreRunnningPrebuilds?: boolean;


Would it make sense to encapsulate these options in a sub-shape ala workspaceStartOptions or so? That would make some of the other code shorter as well, and help keeping this one clean(er).

geropl · 2022-11-04T08:17:13Z

components/server/src/workspace/gitpod-server-impl.ts

@@ -1070,7 +1068,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
            normalizedContextUrl = this.contextParser.normalizeContextURL(contextUrl);
            let runningForContextPromise: Promise<WorkspaceInfo[]> = Promise.resolve([]);
            const contextPromise = this.contextParser.handle(ctx, user, normalizedContextUrl);
-            if (mode === CreateWorkspaceMode.SelectIfRunning) {
+            if (!options.ignoreRunningWorkspaceOnSameCommit) {


geropl · 2022-11-04T08:17:37Z

components/server/src/workspace/gitpod-server-impl.ts

@@ -1153,14 +1151,20 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
                }
            }

-            if (mode === CreateWorkspaceMode.SelectIfRunning && context.forceCreateNewWorkspace !== true) {
+            if (!options.ignoreRunningWorkspaceOnSameCommit && !context.forceCreateNewWorkspace) {


geropl · 2022-11-04T09:09:28Z

components/server/ee/src/workspace/gitpod-server-impl.ts

@@ -998,23 +998,29 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
                prebuiltWorkspace = await this.workspaceDb
                    .trace(ctx)
                    .findPrebuiltWorkspaceByCommit(cloneUrl, commitSHAs);
+                if (prebuiltWorkspace?.state !== "available" && allowUsingPreviousPrebuilds) {


This slight change in behavior (never starting based on previous prebuilds for OpenPrebuildContext) makes sense 💯

Would it make sense to change this to prebuiltWorkspace?.state !== "available" && ignoreRunningPrebuild && allowUsingPreviousPrebuilds?
Otherwise exposing the ignoreRunningPrebuild option to users makes less sense, because it is not independent of allowUsingPreviousPrebuild, and we would have to explain that connection. 😕

geropl · 2022-11-04T09:12:02Z

Code LGTM, starting testing now.

geropl · 2022-11-04T10:00:21Z

/werft run

👍 started the job as gitpod-build-se-inc-ws.3
(with .werft/ from main)

geropl · 2022-11-04T10:26:55Z

Because of the missing preview env and failing build I took the liberty to rebase on latest main with the goal to test...

geropl · 2022-11-04T13:19:03Z

@svenefftinge Can you explain why you think exposing the ignoreRunningWorkspaceOnSameCommit option make sense? To me it feels like an implementation detail of the API, which is offers a nice catch-all for potentially bad UX flows. Or am I missing sth? 🤔

geropl · 2022-11-04T14:37:09Z

I tested back and forth, and it seems to mostly just work (TM) 🙂
Except when "Never wait for running prebuilds" = false && "Allow incremental workspaces" = true I never get to see the prebuild waiting screen.

Update:
Ok, found it here.

geropl · 2022-11-04T15:09:37Z

@svenefftinge Given the last two comments [1, 2] I think it would make sense to just expose a single option: the incremental workspace one. Motivation: the other options are hard to explain, add less additional value, and are not independent.
WDYT?

BTW I'd love to approve a PR that "just" replaces the mode with the option fields, that alone is 🧡 🙃

svenefftinge · 2022-11-04T17:40:17Z

Thanks @geropl ! I've worked a bit on this with @mbrevoort as well and we agreed that we should only have the incremental prebuild setting. I'm moving this to the project level also, to make it easier enabling it for everyone.
We also believe that waiting for non-available prebuilds is never useful, so would like to remove this path entirely. WDYT?

svenefftinge · 2022-11-04T17:44:21Z

@svenefftinge Can you explain why you think exposing the ignoreRunningWorkspaceOnSameCommit option make sense?

Yeah, it doesn't make sense. I think I did it because I could 😅 Have removed it.

geropl · 2022-11-07T08:48:56Z

components/dashboard/src/start/CreateWorkspace.tsx

                    </div>
                </Modal>
            );
-        } else if (result?.runningWorkspacePrebuild) {
-            return (


Can we put this behind a feature flag, just to decouple it from the rollout of the "incremental workspace" feature?

I could make the removal of runningWorkspacePrebuild a separate PR, that we merge first. But I'm not sure that buys us a lot. Would be super helpful to dive into the uncertainty a bit more and figure out if there is something we missed. I can't think of any issues that the removal of this feature could have on existing workflows or deployment.

At the core, the concern I have is: What happens to a high-frequency repo, that did not enable incremental workspaces yet, once we roll this out? To me it feels they'd just never benefit from Prebuilds anymore. Until they understand that there is Incremental Workspaces, and validate that it works for them. This feels like a bad spot for us to be in, and it feels unnecessary as well.

Alternatively, we could leave the screen as-is, maybe tied to !opts.incrementalWorkspace. That way, people can still try to manually fall back to "Use last successful prebuild" (current code) if they want to.
Plus it gives us an obvious spot where to out the advertisement for the "Incremental Workspaces" option, which people can learn about and "migrate" to. 🧘

To me it feels they'd just never benefit from Prebuilds anymore

Why? They would have the same prebuilds as before, just don't get interrupted constantly by a running prebuild when starting a workspace.

Plus it gives us an obvious spot where to out the advertisement for the "Incremental Workspaces" option, which people can learn about and "migrate" to.

I would like to make this the default behavior once we have used it a little and verified it does what we hope it does.

geropl · 2022-11-07T08:53:17Z

we agreed that we should only have the incremental prebuild setting. I'm moving this to the project level also

💯 d'accord: project level also is nice because it_s a property of the project whether it supports incremental build

We also believe that waiting for non-available prebuilds is never useful

@svenefftinge I believe the same, but would be a bit more cautios with rolling out this change. Can we put this behind a feature flag? Just in case there are issues, e.g. unexpecte UX flow bugs etc.? Also, it would feel way better to keep the rollout separate from the other change. 🙂

gtsiolis

Hey everyone! Saw this new setting a left a drive-by comment below which can be also tackled in a follow-up issue. 😁

components/dashboard/src/projects/ProjectSettings.tsx

svenefftinge · 2022-11-07T18:16:55Z

let's go for #14461

roboquat added do-not-merge/work-in-progress release-note-none labels Oct 31, 2022

roboquat added the size/XL label Oct 31, 2022

svenefftinge force-pushed the se/inc-ws branch from 274b9e6 to 1834e8d Compare October 31, 2022 23:12

svenefftinge commented Oct 31, 2022

View reviewed changes

components/dashboard/src/projects/Project.tsx Show resolved Hide resolved

svenefftinge commented Oct 31, 2022

View reviewed changes

svenefftinge marked this pull request as ready for review October 31, 2022 23:59

svenefftinge requested a review from a team October 31, 2022 23:59

roboquat removed the do-not-merge/work-in-progress label Oct 31, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Oct 31, 2022

easyCZ approved these changes Nov 1, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Nov 1, 2022

geropl reviewed Nov 4, 2022

View reviewed changes

geropl force-pushed the se/inc-ws branch from 1834e8d to b7ee4a9 Compare November 4, 2022 10:26

svenefftinge force-pushed the se/inc-ws branch from b7ee4a9 to 80c54c2 Compare November 4, 2022 17:17

svenefftinge marked this pull request as draft November 4, 2022 17:50

roboquat added the do-not-merge/work-in-progress label Nov 4, 2022

svenefftinge force-pushed the se/inc-ws branch 2 times, most recently from 9f52ac1 to 09fcd52 Compare November 6, 2022 20:37

svenefftinge marked this pull request as ready for review November 6, 2022 20:38

roboquat added release-note and removed do-not-merge/work-in-progress release-note-none labels Nov 6, 2022

svenefftinge mentioned this pull request Nov 7, 2022

[server] incremental workspaces #14461

Merged

4 tasks

svenefftinge force-pushed the se/inc-ws branch from 09fcd52 to 327e06f Compare November 7, 2022 03:15

svenefftinge changed the title ~~[server] incremental workspaces~~ [server] incremental workspaces (1 of 2) Nov 7, 2022

geropl reviewed Nov 7, 2022

View reviewed changes

gtsiolis reviewed Nov 7, 2022

View reviewed changes

components/dashboard/src/projects/ProjectSettings.tsx Show resolved Hide resolved

gtsiolis mentioned this pull request Nov 7, 2022

Inform users when a workspace is starting using the last successful prebuild from a previous commit #14479

Open

svenefftinge force-pushed the se/inc-ws branch from 327e06f to dcafc70 Compare November 7, 2022 15:29

roboquat added size/L and removed size/XL labels Nov 7, 2022

Incremental workspaces

7ccdcf4

svenefftinge force-pushed the se/inc-ws branch from dcafc70 to 7ccdcf4 Compare November 7, 2022 15:55

svenefftinge marked this pull request as draft November 7, 2022 18:11

roboquat added the do-not-merge/work-in-progress label Nov 7, 2022

svenefftinge closed this Nov 7, 2022

svenefftinge deleted the se/inc-ws branch December 9, 2022 08:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server] incremental workspaces (1 of 2) #14303

[server] incremental workspaces (1 of 2) #14303

svenefftinge commented Oct 31, 2022 •

edited

Loading

werft-gitpod-dev-com bot commented Oct 31, 2022

svenefftinge Oct 31, 2022

svenefftinge Oct 31, 2022

easyCZ Nov 1, 2022

geropl Nov 4, 2022

svenefftinge Oct 31, 2022

easyCZ left a comment

geropl commented Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl Nov 4, 2022

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022 •

edited

Loading

geropl commented Nov 4, 2022

svenefftinge commented Nov 4, 2022 •

edited

Loading

svenefftinge commented Nov 4, 2022 •

edited

Loading

geropl Nov 7, 2022

svenefftinge Nov 7, 2022

geropl Nov 7, 2022 •

edited

Loading

svenefftinge Nov 7, 2022 •

edited

Loading

svenefftinge Nov 7, 2022

geropl commented Nov 7, 2022

gtsiolis left a comment

svenefftinge commented Nov 7, 2022

		@@ -1065,70 +1067,8 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
		return;
		}

[server] incremental workspaces (1 of 2) #14303

[server] incremental workspaces (1 of 2) #14303

Conversation

svenefftinge commented Oct 31, 2022 • edited Loading

Attention

Description

Related Issue(s)

How to test

Release Notes

Documentation

Werft options:

werft-gitpod-dev-com bot commented Oct 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easyCZ left a comment

Choose a reason for hiding this comment

geropl commented Nov 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022 • edited by werft-gitpod-dev-com bot Loading

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022

geropl commented Nov 4, 2022 • edited Loading

geropl commented Nov 4, 2022

svenefftinge commented Nov 4, 2022 • edited Loading

svenefftinge commented Nov 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geropl Nov 7, 2022 • edited Loading

Choose a reason for hiding this comment

svenefftinge Nov 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geropl commented Nov 7, 2022

gtsiolis left a comment

Choose a reason for hiding this comment

svenefftinge commented Nov 7, 2022

svenefftinge commented Oct 31, 2022 •

edited

Loading

geropl commented Nov 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Nov 4, 2022 •

edited

Loading

svenefftinge commented Nov 4, 2022 •

edited

Loading

svenefftinge commented Nov 4, 2022 •

edited

Loading

geropl Nov 7, 2022 •

edited

Loading

svenefftinge Nov 7, 2022 •

edited

Loading