Simplify prebuild ordering

Author: filiptronicek

components/server/src/prebuilds/incremental-workspace-service.ts

@@ -23,6 +23,12 @@ import { ImageSourceProvider } from "../workspace/image-source-provider";
 
 const MAX_HISTORY_DEPTH = 100;
 
+enum Match {
+    None = 0,
+    Loose = 1,
+    Exact = 2,
+}
+
 @injectable()
 export class IncrementalWorkspaceService {
     @inject(Config) protected readonly config: Config;
@@ -84,88 +90,86 @@ export class IncrementalWorkspaceService {
         // Note: This query returns only not-garbage-collected prebuilds in order to reduce cardinality
         // (e.g., at the time of writing, the Gitpod repository has 16K+ prebuilds, but only ~300 not-garbage-collected)
         const recentPrebuilds = await this.workspaceDB.findPrebuildsWithWorkspace(projectId);
-
-        const sortedRecentPrebuilds = recentPrebuilds
-            .filter((prebuild) => {
-                return history.commitHistory?.includes(prebuild.prebuild.commit);
-            })
-            .sort((a, b) => {
-                // instead of the DB-returned creation time we use the commit history to sort the prebuilds
-                // this way we can return the correct prebuild even if the prebuild was first created for a later commit and then another one for an earlier commit
-                const aIdx = history.commitHistory?.indexOf(a.prebuild.commit) ?? -1;
-                const bIdx = history.commitHistory?.indexOf(b.prebuild.commit) ?? -1;
-
-                return aIdx - bIdx;
-            });
         const imageSource = await imageSourcePromise;
-        for (const recentPrebuild of sortedRecentPrebuilds) {
-            if (
-                this.isGoodBaseforIncrementalBuild(
+
+        // traverse prebuilds by commit history instead of their creationTime, so that we don't match prebuilds created for older revisions but triggered later
+        for (const commit of history.commitHistory) {
+            const prebuildsForCommit = recentPrebuilds.filter(({ prebuild }) => prebuild.commit === commit);
+            for (const entry of prebuildsForCommit) {
+                const { prebuild, workspace } = entry;
+                const match = this.isMatchForIncrementalBuild(
                     history,
                     config,
                     imageSource,
-                    recentPrebuild.prebuild,
-                    recentPrebuild.workspace,
+                    prebuild,
+                    workspace,
                     includeUnfinishedPrebuilds,
-                )
-            ) {
-                return recentPrebuild.prebuild;
+                );
+                if (match > Match.None) {
+                    console.log("Found base for incremental build", {
+                        prebuild,
+                        workspace,
+                        exactMatch: match === Match.Exact,
+                    });
+                    return prebuild;
+                }
             }
         }
 
         return undefined;
     }
 
-    private isGoodBaseforIncrementalBuild(
+    private isMatchForIncrementalBuild(
         history: WithCommitHistory,
         config: WorkspaceConfig,
         imageSource: WorkspaceImageSource,
         candidatePrebuild: PrebuiltWorkspace,
         candidateWorkspace: Workspace,
         includeUnfinishedPrebuilds?: boolean,
-    ): boolean {
-        if (!history.commitHistory || history.commitHistory.length === 0) {
-            return false;
+    ): Match {
+        // make typescript happy, we know that history.commitHistory is defined
+        if (!history.commitHistory) {
+            return Match.None;
         }
         if (!CommitContext.is(candidateWorkspace.context)) {
-            return false;
+            return Match.None;
         }
 
         const acceptableStates: PrebuiltWorkspaceState[] = ["available"];
         if (includeUnfinishedPrebuilds) {
             acceptableStates.push("building");
             acceptableStates.push("queued");
         }
-
         if (!acceptableStates.includes(candidatePrebuild.state)) {
-            return false;
+            return Match.None;
         }
 
-        // we are only considering full prebuilds
-        if (!!candidateWorkspace.basedOnPrebuildId) {
-            return false;
+        // we are only considering full prebuilds (we are not building on top of incremental prebuilds)
+        if (candidateWorkspace.basedOnPrebuildId) {
+            return Match.None;
         }
 
+        // check if the amount of additional repositories matches the candidate
         if (
             candidateWorkspace.context.additionalRepositoryCheckoutInfo?.length !==
             history.additionalRepositoryCommitHistories?.length
         ) {
-            // different number of repos
-            return false;
+            return Match.None;
         }
 
         const candidateCtx = candidateWorkspace.context;
+
+        // check for overlapping commit history
         if (!history.commitHistory.some((sha) => sha === candidateCtx.revision)) {
-            return false;
+            return Match.None;
         }
-
-        // check the commits are included in the commit history
-        for (const subRepo of candidateWorkspace.context.additionalRepositoryCheckoutInfo || []) {
-            const matchIngRepo = history.additionalRepositoryCommitHistories?.find(
+        // check for overlapping git history for each additional repo
+        for (const subRepo of candidateWorkspace.context.additionalRepositoryCheckoutInfo ?? []) {
+            const matchingRepo = history.additionalRepositoryCommitHistories?.find(
                 (repo) => repo.cloneUrl === subRepo.repository.cloneUrl,
             );
-            if (!matchIngRepo || !matchIngRepo.commitHistory.some((sha) => sha === subRepo.revision)) {
-                return false;
+            if (!matchingRepo || !matchingRepo.commitHistory.some((sha) => sha === subRepo.revision)) {
+                return Match.None;
             }
         }
 
@@ -175,29 +179,41 @@ export class IncrementalWorkspaceService {
                 imageSource,
                 parentImageSource: candidateWorkspace.imageSource,
             });
-            return false;
+            return Match.None;
         }
 
         // ensure the tasks haven't changed
-        const filterPrebuildTasks = (tasks: TaskConfig[] = []) =>
-            tasks
-                .map((task) =>
-                    Object.keys(task)
-                        .filter((key) => ["before", "init", "prebuild"].includes(key))
-                        // @ts-ignore
-                        .reduce((obj, key) => ({ ...obj, [key]: task[key] }), {}),
-                )
-                .filter((task) => Object.keys(task).length > 0);
-        const prebuildTasks = filterPrebuildTasks(config.tasks);
-        const parentPrebuildTasks = filterPrebuildTasks(candidateWorkspace.config.tasks);
+        const prebuildTasks = this.filterPrebuildTasks(config.tasks);
+        const parentPrebuildTasks = this.filterPrebuildTasks(candidateWorkspace.config.tasks);
         if (JSON.stringify(prebuildTasks) !== JSON.stringify(parentPrebuildTasks)) {
             log.debug(`Skipping parent prebuild: Outdated prebuild tasks`, {
                 prebuildTasks,
                 parentPrebuildTasks,
             });
-            return false;
+            return Match.None;
+        }
+
+        if (candidatePrebuild.commit === history.commitHistory[0]) {
+            return Match.Exact;
         }
 
-        return true;
+        return Match.Loose;
+    }
+
+    /**
+     * Given an array of tasks returns only the those which are to run during prebuilds, additionally stripping everything besides the prebuild-related configuration from them
+     */
+    private filterPrebuildTasks(tasks: TaskConfig[] = []): Record<string, string>[] {
+        return tasks
+            .map((task) => {
+                const filteredTask: Record<string, any> = {};
+                for (const key of Object.keys(task)) {
+                    if (["before", "init", "prebuild"].includes(key)) {
+                        filteredTask[key] = task[key as keyof TaskConfig];
+                    }
+                }
+                return filteredTask;
+            })
+            .filter((task) => Object.keys(task).length > 0);
     }
 }