From b463ee43dc61646ab02def0e63e51044eb6a847d Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Tue, 19 Oct 2021 14:11:13 +0000 Subject: [PATCH] [admin] Optimize adminGetWorkspaces --- .../dashboard/src/admin/WorkspacesSearch.tsx | 25 ++++++-- .../src/typeorm/workspace-db-impl.ts | 58 +++++++++++-------- .../gitpod-db/src/workspace-db.spec.db.ts | 23 +++++++- components/gitpod-db/src/workspace-db.ts | 4 +- .../gitpod-protocol/src/admin-protocol.ts | 12 +++- .../ee/src/workspace/gitpod-server-impl.ts | 2 +- 6 files changed, 88 insertions(+), 36 deletions(-) diff --git a/components/dashboard/src/admin/WorkspacesSearch.tsx b/components/dashboard/src/admin/WorkspacesSearch.tsx index 794cf5b169fb8c..ebf4ba734287a2 100644 --- a/components/dashboard/src/admin/WorkspacesSearch.tsx +++ b/components/dashboard/src/admin/WorkspacesSearch.tsx @@ -4,7 +4,8 @@ * See License-AGPL.txt in the project root for license information. */ -import { AdminGetListResult, User, WorkspaceAndInstance } from "@gitpod/gitpod-protocol"; +import { AdminGetListResult, AdminGetWorkspacesQuery, User, WorkspaceAndInstance } from "@gitpod/gitpod-protocol"; +import { matchesInstanceIdOrLegacyWorkspaceIdExactly, matchesNewWorkspaceIdExactly } from "@gitpod/gitpod-protocol/lib/util/parse-workspace-id"; import moment from "moment"; import { useContext, useEffect, useState } from "react"; import { useLocation } from "react-router"; @@ -30,7 +31,7 @@ export function WorkspaceSearch(props: Props) { const location = useLocation(); const { user } = useContext(UserContext); const [searchResult, setSearchResult] = useState>({ rows: [], total: 0 }); - const [searchTerm, setSearchTerm] = useState(''); + const [queryTerm, setQueryTerm] = useState(''); const [searching, setSearching] = useState(false); const [currentWorkspace, setCurrentWorkspaceState] = useState(undefined); @@ -67,13 +68,27 @@ export function WorkspaceSearch(props: Props) { const search = async () => { setSearching(true); try { + let searchTerm: string | undefined = queryTerm; + const query: AdminGetWorkspacesQuery = { + ownerId: props?.user?.id, + }; + if (matchesInstanceIdOrLegacyWorkspaceIdExactly(searchTerm)) { + query.instanceIdOrWorkspaceId = searchTerm; + } else if (matchesNewWorkspaceIdExactly(searchTerm)) { + query.workspaceId = searchTerm; + } + if (query.workspaceId || query.instanceId || query.instanceIdOrWorkspaceId) { + searchTerm = undefined; + } + + // const searchTerm = searchTerm; const result = await getGitpodService().server.adminGetWorkspaces({ - searchTerm, limit: 100, orderBy: 'instanceCreationTime', offset: 0, orderDir: "desc", - ownerId: props?.user?.id + ...query, + searchTerm, }); setSearchResult(result); } finally { @@ -89,7 +104,7 @@ export function WorkspaceSearch(props: Props) { - ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} /> + ke.key === 'Enter' && search() } onChange={(v) => { setQueryTerm(v.target.value) }} /> diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index c870dc10fe692e..753d50aedc9079 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -7,7 +7,7 @@ import { injectable, inject } from "inversify"; import { Repository, EntityManager, DeepPartial, UpdateQueryBuilder, Brackets } from "typeorm"; import { MaybeWorkspace, MaybeWorkspaceInstance, WorkspaceDB, FindWorkspacesOptions, PrebuiltUpdatableAndWorkspace, WorkspaceInstanceSessionWithWorkspace, PrebuildWithWorkspace, WorkspaceAndOwner, WorkspacePortsAuthData, WorkspaceOwnerAndSoftDeleted } from "../workspace-db"; -import { Workspace, WorkspaceInstance, WorkspaceInfo, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, RunningWorkspaceInfo, PrebuiltWorkspaceUpdatable, WorkspaceAndInstance, WorkspaceType, PrebuildInfo } from "@gitpod/gitpod-protocol"; +import { Workspace, WorkspaceInstance, WorkspaceInfo, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, RunningWorkspaceInfo, PrebuiltWorkspaceUpdatable, WorkspaceAndInstance, WorkspaceType, PrebuildInfo, AdminGetWorkspacesQuery } from "@gitpod/gitpod-protocol"; import { TypeORM } from "./typeorm"; import { DBWorkspace } from "./entity/db-workspace"; import { DBWorkspaceInstance } from "./entity/db-workspace-instance"; @@ -717,23 +717,37 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { - public async findAllWorkspaceAndInstances(offset: number, limit: number, orderBy: keyof WorkspaceAndInstance, orderDir: "ASC" | "DESC", ownerId?: string, searchTerm?: string): Promise<{ total: number, rows: WorkspaceAndInstance[] }> { - let whereConditions = ['wsi2.id IS NULL']; + public async findAllWorkspaceAndInstances(offset: number, limit: number, orderBy: keyof WorkspaceAndInstance, orderDir: "ASC" | "DESC", query?: AdminGetWorkspacesQuery, searchTerm?: string): Promise<{ total: number, rows: WorkspaceAndInstance[] }> { + let whereConditions = []; let whereConditionParams: any = {}; - - if (!!ownerId) { - // If an owner id is provided only search for workspaces belonging to that user. - whereConditions.push("ws.ownerId = :ownerId"); - whereConditionParams.ownerId = ownerId; + let instanceIdQuery: boolean = false; + + if (query) { + // from most to least specific so we don't generalize accidentally + if (query.instanceIdOrWorkspaceId) { + whereConditions.push("(wsi.id = :instanceId OR ws.id = :workspaceId)"); + whereConditionParams.instanceId = query.instanceIdOrWorkspaceId; + whereConditionParams.workspaceId = query.instanceIdOrWorkspaceId; + } else if (query.instanceId) { + // in addition to adding "instanceId" to the "WHERE" clause like for the other workspace-guided queries, + // we modify the JOIN condition below to a) select the correct instance and b) make the query faster + instanceIdQuery = true; + + whereConditions.push("wsi.id = :instanceId"); + whereConditionParams.instanceId = query.instanceId; + } else if (query.workspaceId) { + whereConditions.push("ws.id = :workspaceId"); + whereConditionParams.workspaceId = query.workspaceId; + } else if (query.ownerId) { + // If an owner id is provided only search for workspaces belonging to that user. + whereConditions.push("ws.ownerId = :ownerId"); + whereConditionParams.ownerId = query.ownerId; + } } - if (!!searchTerm) { + if (searchTerm) { // If a search term is provided perform a wildcard search in the context url or exact match on the workspace id (aka workspace name) or the instance id. - whereConditions.push([ - `ws.contextURL LIKE '%${searchTerm}%'`, - `ws.id = '${searchTerm}'`, - `wsi.id = '${searchTerm}'` - ].join(' OR ')) + whereConditions.push(`ws.contextURL LIKE '%${searchTerm}%'`); } let orderField: string = orderBy; @@ -747,18 +761,16 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { case "ownerId": orderField = "wsi.ownerId"; break; } - // We need to select the latest wsi for a workspace - // see https://stackoverflow.com/questions/2111384/sql-join-selecting-the-last-records-in-a-one-to-many-relationship for how this works + // We need to select the latest wsi for a workspace. It's the same problem we have in 'find' (the "/workspaces" query, see above), so we use the same approach. + // Only twist is that we might be searching for an instance directly ('instanceIdQuery'). const workspaceRepo = await this.getWorkspaceRepo(); let qb = workspaceRepo .createQueryBuilder('ws') - .leftJoinAndMapOne('ws.instance', DBWorkspaceInstance, 'wsi', [ - 'wsi.workspaceId = ws.id' - ].join(' AND ')) - .leftJoinAndMapOne('ignore', DBWorkspaceInstance, 'wsi2', [ - 'wsi2.workspaceId = ws.id', - '(wsi.creationTime < wsi2.creationTime OR (wsi.creationTime = wsi2.creationTime AND wsi.id < wsi2.id))' - ].join(' AND ')) + // We need to put the subquery into the join condition (ON) here to be able to reference `ws.id` which is + // not possible in a subquery on JOIN (e.g. 'LEFT JOIN (SELECT ... WHERE i.workspaceId = ws.id)') + .leftJoinAndMapOne('ws.instance', DBWorkspaceInstance, 'wsi', + `${instanceIdQuery ? "wsi.workspaceId = ws.id" : "wsi.id = (SELECT i.id FROM d_b_workspace_instance AS i WHERE i.workspaceId = ws.id ORDER BY i.creationTime DESC LIMIT 1)"}` + ) .where(whereConditions.join(' AND '), whereConditionParams) .orderBy(orderField, orderDir) .take(limit) diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index 4b2854a8a72ee2..d6a6f1adf7da1d 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -355,7 +355,26 @@ import { DBWorkspaceInstance } from './typeorm/entity/db-workspace-instance'; db.store(this.ws2), db.storeInstance(this.ws2i1), ]); - const dbResult = await db.findAllWorkspaceAndInstances(0, 10, "workspaceId", "DESC", undefined, this.ws2.id); + const dbResult = await db.findAllWorkspaceAndInstances(0, 10, "workspaceId", "DESC", { workspaceId: this.ws2.id }, undefined); + // It should only find one workspace instance + expect(dbResult.total).to.eq(1); + + // It should find the workspace with the queried id + const workspaceAndInstance = dbResult.rows[0] + expect(workspaceAndInstance.workspaceId).to.eq(this.ws2.id) + }); + } + + @test(timeout(10000)) + public async testFindAllWorkspaceAndInstances_workspaceIdOrInstanceId() { + await this.db.transaction(async db => { + await Promise.all([ + db.store(this.ws), + db.storeInstance(this.wsi1), + db.store(this.ws2), + db.storeInstance(this.ws2i1), + ]); + const dbResult = await db.findAllWorkspaceAndInstances(0, 10, "workspaceId", "DESC", { instanceIdOrWorkspaceId: this.ws2.id }, undefined); // It should only find one workspace instance expect(dbResult.total).to.eq(1); @@ -375,7 +394,7 @@ import { DBWorkspaceInstance } from './typeorm/entity/db-workspace-instance'; db.store(this.ws2), db.storeInstance(this.ws2i1), ]); - const dbResult = await db.findAllWorkspaceAndInstances(0, 10, "instanceId", "DESC", undefined, this.wsi1.id); + const dbResult = await this.db.findAllWorkspaceAndInstances(0, 10, "instanceId", "DESC", { instanceId: this.wsi1.id }, undefined); // It should only find one workspace instance expect(dbResult.total).to.eq(1); diff --git a/components/gitpod-db/src/workspace-db.ts b/components/gitpod-db/src/workspace-db.ts index 7233e0c5324585..a4b4b2e70f4c29 100644 --- a/components/gitpod-db/src/workspace-db.ts +++ b/components/gitpod-db/src/workspace-db.ts @@ -6,7 +6,7 @@ import { DeepPartial } from 'typeorm'; -import { Workspace, WorkspaceInfo, WorkspaceInstance, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, PrebuiltWorkspaceUpdatable, RunningWorkspaceInfo, WorkspaceAndInstance, WorkspaceType, PrebuildInfo } from '@gitpod/gitpod-protocol'; +import { Workspace, WorkspaceInfo, WorkspaceInstance, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, PrebuiltWorkspaceUpdatable, RunningWorkspaceInfo, WorkspaceAndInstance, WorkspaceType, PrebuildInfo, AdminGetWorkspacesQuery } from '@gitpod/gitpod-protocol'; export type MaybeWorkspace = Workspace | undefined; export type MaybeWorkspaceInstance = WorkspaceInstance | undefined; @@ -81,7 +81,7 @@ export interface WorkspaceDB { findWorkspacesForContentDeletion(minSoftDeletedTimeInDays: number, limit: number): Promise; findPrebuiltWorkspacesForGC(daysUnused: number, limit: number): Promise; findAllWorkspaces(offset: number, limit: number, orderBy: keyof Workspace, orderDir: "ASC" | "DESC", ownerId?: string, searchTerm?: string, minCreationTime?: Date, maxCreationDateTime?: Date, type?: WorkspaceType): Promise<{ total: number, rows: Workspace[] }>; - findAllWorkspaceAndInstances(offset: number, limit: number, orderBy: keyof WorkspaceAndInstance, orderDir: "ASC" | "DESC", ownerId?: string, searchTerm?: string): Promise<{ total: number, rows: WorkspaceAndInstance[] }>; + findAllWorkspaceAndInstances(offset: number, limit: number, orderBy: keyof WorkspaceAndInstance, orderDir: "ASC" | "DESC", query?: AdminGetWorkspacesQuery, searchTerm?: string): Promise<{ total: number, rows: WorkspaceAndInstance[] }>; findWorkspaceAndInstance(id: string): Promise; findAllWorkspaceInstances(offset: number, limit: number, orderBy: keyof WorkspaceInstance, orderDir: "ASC" | "DESC", ownerId?: string, minCreationTime?: Date, maxCreationTime?: Date, onlyRunning?: boolean, type?: WorkspaceType): Promise<{ total: number, rows: WorkspaceInstance[] }>; diff --git a/components/gitpod-protocol/src/admin-protocol.ts b/components/gitpod-protocol/src/admin-protocol.ts index 68264412016d87..c5281f3515a739 100644 --- a/components/gitpod-protocol/src/admin-protocol.ts +++ b/components/gitpod-protocol/src/admin-protocol.ts @@ -94,6 +94,12 @@ export namespace WorkspaceAndInstance { } } -export interface AdminGetWorkspacesRequest extends AdminGetListRequest { - ownerId?: string -} \ No newline at end of file +export type AdminGetWorkspacesRequest = AdminGetListRequest & AdminGetWorkspacesQuery; +/** The fields are meant to be used either OR (not combined) */ +export type AdminGetWorkspacesQuery = { + /** we use this field in case we have a UUIDv4 and don't know whether it's an (old) workspace or instance id */ + instanceIdOrWorkspaceId?: string; + instanceId?: string; + workspaceId?: string; + ownerId?: string; +}; \ No newline at end of file diff --git a/components/server/ee/src/workspace/gitpod-server-impl.ts b/components/server/ee/src/workspace/gitpod-server-impl.ts index 535978bf8d8675..65e45a22dd819f 100644 --- a/components/server/ee/src/workspace/gitpod-server-impl.ts +++ b/components/server/ee/src/workspace/gitpod-server-impl.ts @@ -605,7 +605,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl