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

Jump to bottom

[admin] Improve workspaces queries #6312

Merged
merged 3 commits into from
Oct 21, 2021
Merged

[admin] Improve workspaces queries #6312

merged 3 commits into from
Oct 21, 2021

Conversation

geropl
Copy link
Member

@geropl geropl commented Oct 20, 2021 •
edited
Loading

Description

Improves performance of findAllWorkspaceAndInstances by being more explicit what we ask for

Related Issue(s)

fixes the workspace part of #4988

How to test

unit test are fine

  • see the build

manual test positive

Release Notes

`/admin`: Improve performance of workspace queries

Documentation

@geropl
[protocol] Add matchesInstanceIdExactly and matchesWorkspaceIdExactly
a524cff
@geropl
[admin] Optimize adminGetWorkspaces
f8f559f
@geropl
[db] Make workspace DB tests run reliably
edfaf5b 
At some point we used db.transaction(...) for all tests, which is not necessary, and also breaks certain queries
@geropl geropl mentioned this pull request Oct 20, 2021
Closed
@geropl geropl marked this pull request as ready for review October 20, 2021 07:48
geropl
geropl commented Oct 20, 2021
View reviewed changes
components/dashboard/src/admin/WorkspacesSearch.tsx
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.

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Oct 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-gpl-4988-admin-queries.4

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Oct 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-gpl-4988-admin-queries.5

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Oct 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-gpl-4988-admin-queries.6

@geropl
Copy link
Member Author

geropl commented Oct 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-gpl-4988-admin-queries.7

@JanKoehnlein
Copy link
Contributor

Is it expected that only the current instance of a WS can be queried?

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Oct 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-gpl-4988-admin-queries.8

@JanKoehnlein
Copy link
Contributor

/lgtm

@roboquat roboquat added the lgtm label Oct 21, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 1a4874be2043ab7f19ce82a6f2d3349e3e307a6f

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JanKoehnlein

Associated issue: #4988

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit de77303 into main Oct 21, 2021
@roboquat roboquat deleted the gpl/4988-admin-queries branch October 21, 2021 07:04
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexTugarev AlexTugarev Awaiting requested review from AlexTugarev

@gtsiolis gtsiolis Awaiting requested review from gtsiolis

Assignees

@JanKoehnlein JanKoehnlein

Labels
approved deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/XXL team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@geropl @JanKoehnlein @roboquat