Skip to content

Commit

Permalink
chore: refactor workspaces query to use window function (#5079)
Browse files Browse the repository at this point in the history
* Use window function in query

* Convert workspace rows and unpack count

* Update types

* Fix Scan bug

* Remove getCountError
  • Loading branch information
presleyp committed Nov 16, 2022
1 parent 560d3c9 commit e6ead7d
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 41 deletions.
3 changes: 2 additions & 1 deletion coderd/autobuild/executor/lifecycle_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
37 changes: 30 additions & 7 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 21 additions & 0 deletions coderd/database/modelmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 5 additions & 4 deletions coderd/database/modelqueries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion coderd/database/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 20 additions & 4 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion coderd/database/queries/workspaces.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ LIMIT

-- name: GetWorkspaces :many
SELECT
workspaces.*
workspaces.*, COUNT(*) OVER () as count
FROM
workspaces
LEFT JOIN LATERAL (
Expand Down
3 changes: 2 additions & 1 deletion coderd/telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
20 changes: 8 additions & 12 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,23 @@ 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.",
Detail: err.Error(),
})
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 {
Expand All @@ -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),
})
}

Expand Down
4 changes: 1 addition & 3 deletions site/src/pages/WorkspacesPage/WorkspacesPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -44,7 +43,6 @@ const WorkspacesPage: FC = () => {
workspaceRefs={workspaceRefs}
count={count}
getWorkspacesError={getWorkspacesError}
getCountError={getCountError}
onFilter={(query) => {
send({
type: "UPDATE_FILTER",
Expand Down
6 changes: 0 additions & 6 deletions site/src/pages/WorkspacesPage/WorkspacesPageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -46,7 +45,6 @@ export const WorkspacesPageView: FC<
workspaceRefs,
count,
getWorkspacesError,
getCountError,
filter,
onFilter,
paginationRef,
Expand Down Expand Up @@ -92,10 +90,6 @@ export const WorkspacesPageView: FC<
/>
</Maybe>

<Maybe condition={getCountError !== undefined}>
<AlertBanner error={getCountError} severity="warning" />
</Maybe>

<SearchBarWithFilter
filter={filter}
onFilter={onFilter}
Expand Down
1 change: 0 additions & 1 deletion site/src/xServices/workspaces/workspacesXService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ interface WorkspacesContext {
filter: string
count?: number
getWorkspacesError?: Error | unknown
getCountError?: Error | unknown
paginationContext: PaginationContext
}

Expand Down

0 comments on commit e6ead7d

Please sign in to comment.