From 7eaae28dac2f727fbeba24e5bf3aa1b4794737f0 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Mon, 14 Nov 2022 19:59:44 +0000 Subject: [PATCH 1/5] Use window function in query --- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 24 ++++++++++++++++++++---- coderd/database/queries/workspaces.sql | 2 +- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 7320ea523683d..a423c2a360151 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -123,7 +123,7 @@ type sqlcQuerier interface { GetWorkspaceResourcesByJobID(ctx context.Context, jobID uuid.UUID) ([]WorkspaceResource, error) GetWorkspaceResourcesByJobIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceResource, error) GetWorkspaceResourcesCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceResource, error) - GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]Workspace, error) + GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) InsertAgentStat(ctx context.Context, arg InsertAgentStatParams) (AgentStat, error) // We use the organization_id as the id diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9a298dc0053cd..7a3bf6e6d6d91 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6196,7 +6196,7 @@ func (q *sqlQuerier) GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, i const getWorkspaces = `-- name: GetWorkspaces :many SELECT - workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at + workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, COUNT(*) OVER () as count FROM workspaces LEFT JOIN LATERAL ( @@ -6343,7 +6343,22 @@ type GetWorkspacesParams struct { Limit int32 `db:"limit_" json:"limit_"` } -func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]Workspace, error) { +type GetWorkspacesRow struct { + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + Deleted bool `db:"deleted" json:"deleted"` + Name string `db:"name" json:"name"` + AutostartSchedule sql.NullString `db:"autostart_schedule" json:"autostart_schedule"` + Ttl sql.NullInt64 `db:"ttl" json:"ttl"` + LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` + Count int64 `db:"count" json:"count"` +} + +func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) { rows, err := q.db.QueryContext(ctx, getWorkspaces, arg.Deleted, arg.Status, @@ -6359,9 +6374,9 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) return nil, err } defer rows.Close() - var items []Workspace + var items []GetWorkspacesRow for rows.Next() { - var i Workspace + var i GetWorkspacesRow if err := rows.Scan( &i.ID, &i.CreatedAt, @@ -6374,6 +6389,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.AutostartSchedule, &i.Ttl, &i.LastUsedAt, + &i.Count, ); err != nil { return nil, err } diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 6e07827c10d4a..65815c9af9ddd 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -10,7 +10,7 @@ LIMIT -- name: GetWorkspaces :many SELECT - workspaces.* + workspaces.*, COUNT(*) OVER () as count FROM workspaces LEFT JOIN LATERAL ( From 3d9e2cfb5f04af26658189dc6720e3697479fec2 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Mon, 14 Nov 2022 20:18:04 +0000 Subject: [PATCH 2/5] Convert workspace rows and unpack count --- coderd/database/modelmethods.go | 21 +++++++++++++++++++++ coderd/database/modelqueries.go | 8 ++++---- coderd/workspaces.go | 20 ++++++++------------ 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index b9e9bc57e5626..5257bec1c3a2f 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -93,3 +93,24 @@ func ConvertUserRows(rows []GetUsersRow) []User { return users } + +func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace { + workspaces := make([]Workspace, len(rows)) + for i, r := range rows { + workspaces[i] = Workspace{ + ID: r.ID, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, + OwnerID: r.OwnerID, + OrganizationID: r.OrganizationID, + TemplateID: r.TemplateID, + Deleted: r.Deleted, + Name: r.Name, + AutostartSchedule: r.AutostartSchedule, + Ttl: r.Ttl, + LastUsedAt: r.LastUsedAt, + } + } + + return workspaces +} diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index e27d347807c62..d18d4376308c6 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -112,13 +112,13 @@ func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([ } type workspaceQuerier interface { - GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) + GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]GetWorkspacesRow, error) } // GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access. // This code is copied from `GetWorkspaces` and adds the authorized filter WHERE // clause. -func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) { +func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]GetWorkspacesRow, error) { // In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the // authorizedFilter between the end of the where clause and those statements. filter := strings.Replace(getWorkspaces, "-- @authorize_filter", fmt.Sprintf(" AND %s", authorizedFilter.SQLString(rbac.NoACLConfig())), 1) @@ -139,9 +139,9 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa return nil, xerrors.Errorf("get authorized workspaces: %w", err) } defer rows.Close() - var items []Workspace + var items []GetWorkspacesRow for rows.Next() { - var i Workspace + var i GetWorkspacesRow if err := rows.Scan( &i.ID, &i.CreatedAt, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 540fd42665761..04ad0b078dd86 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -127,7 +127,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { return } - workspaces, err := api.Database.GetAuthorizedWorkspaces(ctx, filter, sqlFilter) + workspaceRows, err := api.Database.GetAuthorizedWorkspaces(ctx, filter, sqlFilter) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspaces.", @@ -135,19 +135,15 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { }) return } - - // run the query again to get the total count for frontend pagination - filter.Offset = 0 - filter.Limit = 0 - all, err := api.Database.GetAuthorizedWorkspaces(ctx, filter, sqlFilter) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspaces.", - Detail: err.Error(), + if len(workspaceRows) == 0 { + httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{ + Workspaces: []codersdk.Workspace{}, + Count: 0, }) return } - count := len(all) + + workspaces := database.ConvertWorkspaceRows(workspaceRows) data, err := api.workspaceData(ctx, workspaces) if err != nil { @@ -169,7 +165,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{ Workspaces: wss, - Count: count, + Count: int(workspaceRows[0].Count), }) } From c9b36dceaf4edd9154a1b24a9da20d26398525ef Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 15 Nov 2022 02:04:40 +0000 Subject: [PATCH 3/5] Update types --- .../autobuild/executor/lifecycle_executor.go | 3 +- coderd/database/databasefake/databasefake.go | 37 +++++++++++++++---- coderd/telemetry/telemetry.go | 3 +- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 90126d074b190..ba2795a3a202f 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -99,13 +99,14 @@ func (e *Executor) runOnce(t time.Time) Stats { // NOTE: If a workspace build is created with a given TTL and then the user either // changes or unsets the TTL, the deadline for the workspace build will not // have changed. This behavior is as expected per #2229. - workspaces, err := e.db.GetWorkspaces(e.ctx, database.GetWorkspacesParams{ + workspaceRows, err := e.db.GetWorkspaces(e.ctx, database.GetWorkspacesParams{ Deleted: false, }) if err != nil { e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err)) return stats } + workspaces := database.ConvertWorkspaceRows(workspaceRows) var eligibleWorkspaceIDs []uuid.UUID for _, ws := range workspaces { diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index ae4c2fafd8760..6ca1b365f56d3 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -718,14 +718,14 @@ func (q *fakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U }, nil } -func (q *fakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.Workspace, error) { +func (q *fakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { // A nil auth filter means no auth filter. - workspaces, err := q.GetAuthorizedWorkspaces(ctx, arg, nil) - return workspaces, err + workspaceRows, err := q.GetAuthorizedWorkspaces(ctx, arg, nil) + return workspaceRows, err } //nolint:gocyclo -func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]database.Workspace, error) { +func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]database.GetWorkspacesRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -866,20 +866,43 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. workspaces = append(workspaces, workspace) } + beforePageCount := len(workspaces) + if arg.Offset > 0 { if int(arg.Offset) > len(workspaces) { - return []database.Workspace{}, nil + return []database.GetWorkspacesRow{}, nil } workspaces = workspaces[arg.Offset:] } if arg.Limit > 0 { if int(arg.Limit) > len(workspaces) { - return workspaces, nil + return convertToWorkspaceRows(workspaces, int64(beforePageCount)), nil } workspaces = workspaces[:arg.Limit] } - return workspaces, nil + return convertToWorkspaceRows(workspaces, int64(beforePageCount)), nil +} + +func convertToWorkspaceRows(workspaces []database.Workspace, count int64) []database.GetWorkspacesRow { + rows := make([]database.GetWorkspacesRow, len(workspaces)) + for i, w := range workspaces { + rows[i] = database.GetWorkspacesRow{ + ID: w.ID, + CreatedAt: w.CreatedAt, + UpdatedAt: w.UpdatedAt, + OwnerID: w.OwnerID, + OrganizationID: w.OrganizationID, + TemplateID: w.TemplateID, + Deleted: w.Deleted, + Name: w.Name, + AutostartSchedule: w.AutostartSchedule, + Ttl: w.Ttl, + LastUsedAt: w.LastUsedAt, + Count: count, + } + } + return rows } func (q *fakeQuerier) GetWorkspaceByID(_ context.Context, id uuid.UUID) (database.Workspace, error) { diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go index 88b72b86af67f..0db7464771274 100644 --- a/coderd/telemetry/telemetry.go +++ b/coderd/telemetry/telemetry.go @@ -380,10 +380,11 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) { return nil }) eg.Go(func() error { - workspaces, err := r.options.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{}) + workspaceRows, err := r.options.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{}) if err != nil { return xerrors.Errorf("get workspaces: %w", err) } + workspaces := database.ConvertWorkspaceRows(workspaceRows) snapshot.Workspaces = make([]Workspace, 0, len(workspaces)) for _, dbWorkspace := range workspaces { snapshot.Workspaces = append(snapshot.Workspaces, ConvertWorkspace(dbWorkspace)) From 9b2964f88fd2c328b7e5edd0ae255d6f4a94cf86 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 15 Nov 2022 02:38:27 +0000 Subject: [PATCH 4/5] Fix Scan bug --- coderd/database/modelqueries.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index d18d4376308c6..0e553c601367c 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -154,6 +154,7 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa &i.AutostartSchedule, &i.Ttl, &i.LastUsedAt, + &i.Count, ); err != nil { return nil, err } From 66d35e393d4f413757b143cd6bf31e4e5a3edd4f Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Wed, 16 Nov 2022 14:58:38 +0000 Subject: [PATCH 5/5] Remove getCountError --- site/src/pages/WorkspacesPage/WorkspacesPage.tsx | 4 +--- site/src/pages/WorkspacesPage/WorkspacesPageView.tsx | 6 ------ site/src/xServices/workspaces/workspacesXService.ts | 1 - 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 1171cc99cbf46..6a9e1197ac58d 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -27,8 +27,7 @@ const WorkspacesPage: FC = () => { }, }) - const { workspaceRefs, count, getWorkspacesError, getCountError } = - workspacesState.context + const { workspaceRefs, count, getWorkspacesError } = workspacesState.context const paginationRef = workspacesState.context .paginationRef as PaginationMachineRef @@ -44,7 +43,6 @@ const WorkspacesPage: FC = () => { workspaceRefs={workspaceRefs} count={count} getWorkspacesError={getWorkspacesError} - getCountError={getCountError} onFilter={(query) => { send({ type: "UPDATE_FILTER", diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 7eeafb6f514b7..7532c3aa4d816 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -32,7 +32,6 @@ export interface WorkspacesPageViewProps { workspaceRefs?: WorkspaceItemMachineRef[] count?: number getWorkspacesError: Error | unknown - getCountError: Error | unknown filter?: string onFilter: (query: string) => void paginationRef: PaginationMachineRef @@ -46,7 +45,6 @@ export const WorkspacesPageView: FC< workspaceRefs, count, getWorkspacesError, - getCountError, filter, onFilter, paginationRef, @@ -92,10 +90,6 @@ export const WorkspacesPageView: FC< /> - - - -