8000 Filter query: has-agent connecting, connected, disconnected, timeout by mtojek · Pull Request #5145 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

Filter query: has-agent connecting, connected, disconnected, timeout #5145

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

Merged
merged 30 commits into from
Nov 24, 2022
Merged

Filter query: has-agent connecting, connected, disconnected, timeout #5145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ee6577d
WIP
mtojek Nov 21, 2022
7d61519
has-agent:connecting, connected
mtojek Nov 22, 2022
b5a1ecc
Fix
mtojek Nov 22, 2022
dca2b8b
Fix
mtojek Nov 22, 2022
0a4746c
has-agent:disconnected, timeout
mtojek Nov 22, 2022
70952e1
Fix: typo
mtojek Nov 22, 2022
7965a54
Fix
mtojek Nov 22, 2022
c1bd839
TODOs
mtojek Nov 22, 2022
2af4133
databasefake
mtojek Nov 22, 2022
f9e2167
Fix: typo
mtojek Nov 22, 2022
45957c1
More TODOs
mtojek Nov 22, 2022
66892d7
databasefake
mtojek Nov 22, 2022
fc93f90
Timeout tests
mtojek Nov 22, 2022
< 8000 a href="/coder/coder/pull/5145/commits/e587b5d555d2a3b40930838f110d8375f5676013" class="select-menu-item " role="menuitem" data-commit="e587b5d555d2a3b40930838f110d8375f5676013" data-parent-commit="fc93f906897412995246b5e5910de28eed3d32fa">
e587b5d
Address PR comments
mtojek Nov 22, 2022
458a8eb
Implement FIXMEs
mtojek Nov 23, 2022
81d9fef
Renamings
mtojek Nov 23, 2022
5ca8c17
Address PR comments
mtojek Nov 23, 2022
ec2d571
Fix: readability
mtojek Nov 23, 2022
4b2f831
Fix: refactor CASE logic
mtojek Nov 23, 2022
3c0d4fe
CASE logic
mtojek Nov 23, 2022
9067a9e
Fix
mtojek Nov 23, 2022
c14663c
Use CTE
mtojek Nov 23, 2022
9c2f64a
Polishing
mtojek Nov 23, 2022
44bf343
Comment
mtojek Nov 24, 2022
598b140
WIP
mtojek Nov 24, 2022
f3787d1
Merge branch 'main' into 4679-surface-agent
mtojek Nov 24, 2022
ba13e03
IS NOT NULL
mtojek Nov 24, 2022
d74b072
Without CTE
mtojek Nov 24, 2022
4255448
One more optimization
mtojek Nov 24, 2022
4ace659
2nd optimization
mtojek Nov 24, 2022
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
Prev Previous commit
Next Next commit
Use CTE
  • Loading branch information
@mtojek
mtojek committed Nov 23, 2022
commit c14663c0c7dab22d94f0b03788ec42fc617d017e
2 changes: 1 addition & 1 deletion coderd/database/modelqueries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
pq.Array(arg.TemplateIds),
arg.Name,
arg.HasAgent,
arg.AgentInactiveDisconnectTimeoutSeconds,
arg.Offset,
arg.Limit,
arg.AgentInactiveDisconnectTimeoutSeconds,
)
if err != nil {
return nil, xerrors.Errorf("get authorized workspaces: %w", err)
Expand Down
103 changes: 58 additions & 45 deletions coderd/database/queries.sql.go
View file
Open in desktop

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

93 changes: 53 additions & 40 deletions coderd/database/queries/workspaces.sql
View file
Open in desktop
42D6
Original file line number Diff line number Diff line change
Expand Up @@ -38,47 +38,72 @@ WHERE
);

-- name: GetWorkspaces :many
WITH workspace_builds_agents AS (
Copy link
Member

Choose a reason for hiding this comment

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

If you also CTE'd the latest_build query, like this:

WITH latest_build AS (
	SELECT ...
), workspace_builds_agents AS (
	SELECT ...
)

You could join latest_builds in workspace_builds_agents to reduce the agent results to the latest build only. This should make the query more performant (reduced rows).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good suggestion, but won't it be a problem that latest_build goes over all workspaces?

SELECT
	workspaces.*, COUNT(*) OVER () as count
FROM
	workspaces
LEFT JOIN LATERAL (
	SELECT

...

	WHERE
		workspace_builds.workspace_id = workspaces.id
...
) latest_build ON TRUE

I presume that we will need latest_builds (plural) for every workspace, but I'm not sure if it isn't the same complexity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I investigated a few concepts we talked about offline with @mafredri:

  1. Replace latest_build with workspace_latest_builds CTE.

Unfortunately, I wasn't able to select only the last build per workspace.

  1. Replace workspace_build_agent CTE with JOIN LATERAL.

Workspaces with multiple agents will return multiple records, which is unexpected.

  1. Place the CASE logic in the WHERE clause

It looks like the conditional logic will fire only if has-agent is specified and it shouldn't affect other GetWorkspaces queries.

Let me know your thoughts.

SELECT
workspace_builds.workspace_id AS workspace_id,
workspace_builds.build_number AS build_number,
workspace_agents.id AS agent_id,
(
CASE
WHEN workspace_agents.first_connected_at IS NULL THEN
CASE
WHEN workspace_agents.connection_timeout_seconds > 0 AND NOW() - workspace_agents.created_at > workspace_agents.connection_timeout_seconds * INTERVAL '1 second' THEN
'timeout'
ELSE
'connecting'
END
WHEN workspace_agents.disconnected_at > workspace_agents.last_connected_at THEN
'disconnected'
WHEN NOW() - workspace_agents.last_connected_at > INTERVAL '1 second' * @agent_inactive_disconnect_timeout_seconds :: bigint THEN
'disconnected'
WHEN workspace_agents.last_connected_at IS NOT NULL THEN
'connected'
ELSE
NULL
END
) AS agent_status
FROM
workspace_builds
LEFT JOIN
provisioner_jobs
ON
provisioner_jobs.id = workspace_builds.job_id
LEFT JOIN
workspace_resources
ON
workspace_resources.job_id = provisioner_jobs.id
LEFT JOIN
workspace_agents
ON
workspace_agents.resource_id = workspace_resources.id
WHERE
workspace_builds.transition = 'start'::workspace_transition AND
workspace_agents.id IS NOT NULL
)
SELECT
workspaces.*, COUNT(*) OVER () as count
FROM
workspaces
LEFT JOIN LATERAL (
SELECT
build_number,
workspace_builds.job_id,
workspace_builds.build_number,
workspace_builds.transition,
provisioner_jobs.started_at,
provisioner_jobs.updated_at,
provisioner_jobs.canceled_at,
provisioner_jobs.completed_at,
provisioner_jobs.error,
workspace_agents.created_at AS agent_created_at,
workspace_agents.disconnected_at AS agent_disconnected_at,
workspace_agents.first_connected_at AS agent_first_connected_at,
workspace_agents.last_connected_at AS agent_last_connected_at,
workspace_agents.connection_timeout_seconds AS agent_connection_timeout_seconds
provisioner_jobs.error
FROM
workspace_builds
LEFT JOIN
provisioner_jobs
ON
provisioner_jobs.id = workspace_builds.job_id
LEFT JOIN
workspace_resources
ON
workspace_resources.job_id = provisioner_jobs.id
LEFT JOIN
workspace_agents
ON
workspace_agents.resource_id = workspace_resources.id
WHERE
workspace_builds.workspace_id = workspaces.id
ORDER BY
build_number DESC,
agent_last_connected_at ASC,
agent_first_connected_at ASC
LIMIT
1
build_number DESC
LIMIT 1
) latest_build ON TRUE
WHERE
-- Optionally include deleted workspaces
Expand Down Expand Up @@ -180,27 +205,15 @@ WHERE
END
-- Filter by agent status
-- has-agent: is only applicable for workspaces in "start" transition. Stopped and deleted workspaces don't have agents.
-- The following CASE statement reflects the conditional logic in coderd/workspaceagents.go
AND CASE
WHEN @has_agent :: text != '' THEN
latest_build.transition = 'start'::workspace_transition
AND CASE
WHEN latest_build.agent_first_connected_at IS NULL THEN
CASE
WHEN latest_build.agent_connection_timeout_seconds > 0 AND NOW() - latest_build.agent_created_at > latest_build.agent_connection_timeout_seconds * INTERVAL '1 second' THEN
CASE WHEN @has_agent :: text = 'timeout' THEN true ELSE false END
ELSE
CASE WHEN @has_agent :: text = 'connecting' THEN true ELSE false END
END
WHEN latest_build.agent_disconnected_at > latest_build.agent_last_connected_at THEN
CASE WHEN @has_agent :: text = 'disconnected' THEN true ELSE false END
WHEN NOW() - latest_build.agent_last_connected_at > INTERVAL '1 second' * @agent_inactive_disconnect_timeout_seconds :: bigint THEN
CASE WHEN @has_agent :: text = 'disconnected' THEN true ELSE false END
WHEN latest_build.agent_last_connected_at IS NOT NULL THEN
CASE WHEN @has_agent :: text = 'connected' THEN true ELSE false END
ELSE
false
END
(
SELECT COUNT(*) FROM workspace_builds_agents
WHERE
workspace_builds_agents.workspace_id = workspaces.id AND
workspace_builds_agents.build_number = latest_build.build_number AND
agent_status = @has_agent
) > 0
ELSE true
END
-- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces
Expand Down
0