NotificationsYou must be signed in to change notification settings
Fork1k
Star11.1k

chore(coderd/database): optimize GetRunningPrebuiltWorkspaces#18588

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

johnstcn merged 3 commits intomainfromcj/optimize/GetRunningPrebuiltWorkspaces

Jul 1, 2025

Merged

chore(coderd/database): optimize GetRunningPrebuiltWorkspaces#18588

johnstcn merged 3 commits intomainfromcj/optimize/GetRunningPrebuiltWorkspaces

Jul 1, 2025

+188 −25

Conversation

Copy link

Member

johnstcn commentedJun 25, 2025•
edited
Loading

Fixescoder/internal#715

After this change, the only use of theworkspace_prebuilds view is theClaimPrebuiltWorkspace query. A subsequent PR will update the view.

Before: ~44mshttps://explain.dalibo.com/plan/76cbe21d1a4c9329#plan

~After:~~3.7mshttps://explain.dalibo.com/plan/f61beg93f9306fc0#plan~~

After: 7.3mshttps://explain.dalibo.com/plan/5abbdf926315677e#plan

github-actionsbot assignedjohnstcn

Jun 25, 2025

johnstcn marked this pull request as ready for review

June 26, 2025 20:50

johnstcn requested review frommafredri andSasSwart

June 26, 2025 20:50

mafredri reviewed

Jun 27, 2025

View reviewed changes

Copy link

Member

mafredri left a comment

Choose a reason for hiding this comment

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

I was a bit thrown off by the indentation, but I tried my best 😆, left some suggestions and questions.

coderd/database/queries/prebuilds.sql OutdatedShow resolvedHide resolved

coderd/database/queries/prebuilds.sql Outdated

		AS (
		SELECT
		latest_prebuilds.job_id,
		BOOL_AND(workspace_agents.lifecycle_state='ready'::workspace_agent_lifecycle_state)::booleanAS ready

Copy link

Member

Choose a reason for hiding this comment

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

Suggested change

	BOOL_AND(workspace_agents.lifecycle_state='ready'::workspace_agent_lifecycle_state)::booleanAS ready
	workspace_agents.lifecycle_state='ready'::workspace_agent_lifecycle_stateAS ready

What's this, it actually works?

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

I just copied it from theworkspace_prebuilds view.

coderd/database/queries/prebuilds.sql Outdated

		latest_prebuilds.created_at
		FROM
		latest_prebuilds
		LEFT JOIN ready_agentsON

Copy link

Member

Choose a reason for hiding this comment

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

This join will duplicate rows when there are multiple agents since there is no group-by here or in theready_agents CTE, was this considered?

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

That's a good point. I'll add some unit tests to compare the behaviour of the previous query.

Copy link

MemberAuthor

johnstcnJun 30, 2025•
edited
Loading

Choose a reason for hiding this comment

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

The 'correct' behaviour seems to be one row per provisioner job / workspace build.

coderd/database/queries/prebuilds.sql OutdatedShow resolvedHide resolved

johnstcn added2 commits

June 30, 2025 12:22

chore(coderd/database): add test GetRunningPrebuiltWorkspaces

02d1576

chore(coderd/database): optimize GetRunningPrebuiltWorkspaces

9131d78

johnstcn force-pushed thecj/optimize/GetRunningPrebuiltWorkspaces branch fromab80dc0 to9131d78Compare

June 30, 2025 11:23

johnstcn commented

Jun 30, 2025

View reviewed changes

coderd/database/querier_test.go

		}
		}

		funcTestGetRunningPrebuiltWorkspaces(t*testing.T) {

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

reivew: adapted fromTestWorkspacePrebuildsView.

mafredri approved these changes

Jun 30, 2025

View reviewed changes

Copy link

Member

mafredri left a comment

Choose a reason for hiding this comment

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

Would love to see an updated plan but looks good, nice work on the optimization!

coderd/database/queries/prebuilds.sql OutdatedShow resolvedHide resolved

coderd/database/queries/prebuilds.sql

		latest_prebuilds.created_at
		FROM latest_prebuilds
		LEFT JOIN ready_agentsONready_agents.job_id=latest_prebuilds.job_id
		ORDER BYlatest_prebuilds.workspace_idASC;

Copy link

Member

Choose a reason for hiding this comment

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

Nice, formatting ischef's kiss

coderd/database/queries/prebuilds.sql OutdatedShow resolvedHide resolved

Copy link

MemberAuthor

johnstcn commentedJun 30, 2025

The updated plan is slower than the previous one, but correct according to the added unit tests.

address comments

5635a41

Copy link

Member

mafredri commentedJun 30, 2025

The updated plan is slower than the previous one, but correct according to the added unit tests.

Interesting, it could be due to moving WHERE conditions. It's always interesting how a hypothesis for optimization can crumble in the face of actual data layout. And where naive improvements may actually stomp on the query planners feet. 😅 Either way, the "slower" improvement is still great, no need to micro optimize IMO. 👍🏻