Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[admin] Improve workspaces queries #6312

Merged
merged 3 commits into from
Oct 21, 2021
Merged

[admin] Improve workspaces queries #6312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a524cff
[protocol] Add matchesInstanceIdExactly and matchesWorkspaceIdExactly
geropl Oct 19, 2021
f8f559f
[admin] Optimize adminGetWorkspaces
geropl Oct 19, 2021
edfaf5b
[db] Make workspace DB tests run reliably
geropl Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions components/dashboard/src/admin/WorkspacesSearch.tsx
View file
Open in desktop
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)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gtsiolis What I try to do here is to "guess" what the user is entering, to enable performance improvements in the SQL query. Problem is that this is not perfect (cmp. instanceIdOrWorkspaceId). Another option would be a Dropdown of sorts that allows to choose this explicitly.
I would be interested in your perspective on this; maybe you have an idea how to improve here without requiring too much friction.

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
Open in desktop
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
Loading