Skip to content

Commit

Permalink
[admin] Optimize adminGetWorkspaces
Browse files Browse the repository at this point in the history
  • Loading branch information
@geropl @roboquat
geropl authored and roboquat committed Oct 21, 2021
1 parent 1ac7851 commit b463ee4
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 36 deletions.
25 changes: 20 additions & 5 deletions components/dashboard/src/admin/WorkspacesSearch.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -30,7 +31,7 @@ export function WorkspaceSearch(props: Props) {
const location = useLocation();
const { user } = useContext(UserContext);
const [searchResult, setSearchResult] = useState<AdminGetListResult<WorkspaceAndInstance>>({ rows: [], total: 0 });
const [searchTerm, setSearchTerm] = useState('');
const [queryTerm, setQueryTerm] = useState('');
const [searching, setSearching] = useState(false);
const [currentWorkspace, setCurrentWorkspaceState] = useState<WorkspaceAndInstance|undefined>(undefined);

Expand Down Expand Up @@ -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 {
Expand All @@ -89,7 +104,7 @@ export function WorkspaceSearch(props: Props) {
<path fillRule="evenodd" clipRule="evenodd" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z" fill="#A8A29E" />
</svg>
</div>
<input type="search" placeholder="Search Workspaces" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} />
<input type="search" placeholder="Search Workspaces" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setQueryTerm(v.target.value) }} />
</div>
<button disabled={searching} onClick={search}>Search</button>
</div>
Expand Down
58 changes: 35 additions & 23 deletions components/gitpod-db/src/typeorm/workspace-db-impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
23 changes: 21 additions & 2 deletions components/gitpod-db/src/workspace-db.spec.db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions components/gitpod-db/src/workspace-db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -81,7 +81,7 @@ export interface WorkspaceDB {
findWorkspacesForContentDeletion(minSoftDeletedTimeInDays: number, limit: number): Promise<WorkspaceOwnerAndSoftDeleted[]>;
findPrebuiltWorkspacesForGC(daysUnused: number, limit: number): Promise<WorkspaceAndOwner[]>;
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<WorkspaceAndInstance | undefined>;

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[] }>;
Expand Down
12 changes: 9 additions & 3 deletions components/gitpod-protocol/src/admin-protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ export namespace WorkspaceAndInstance {
}
}

export interface AdminGetWorkspacesRequest extends AdminGetListRequest<WorkspaceAndInstance> {
ownerId?: string
}
export type AdminGetWorkspacesRequest = AdminGetListRequest<WorkspaceAndInstance> & 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;
};
2 changes: 1 addition & 1 deletion components/server/ee/src/workspace/gitpod-server-impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl<GitpodClient, GitpodSer

const span = opentracing.globalTracer().startSpan("adminGetWorkspaces");
try {
return await this.workspaceDb.trace({ span }).findAllWorkspaceAndInstances(req.offset, req.limit, req.orderBy, req.orderDir === "asc" ? "ASC" : "DESC", req.ownerId, req.searchTerm);
return await this.workspaceDb.trace({ span }).findAllWorkspaceAndInstances(req.offset, req.limit, req.orderBy, req.orderDir === "asc" ? "ASC" : "DESC", req, req.searchTerm);
} catch (e) {
TraceContext.logError({ span }, e);
throw new ResponseError(500, e.toString());
Expand Down

0 comments on commit b463ee4

Please sign in to comment.