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.

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

fix: GetActiveDeployments fix #2654

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Conversation

stuartwdouglas
Copy link
Collaborator

No description provided.

@stuartwdouglas stuartwdouglas requested review from a team and worstell and removed request for a team September 12, 2024 02:36
This was referenced Sep 12, 2024
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Sep 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 12, 2024
@stuartwdouglas stuartwdouglas added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Sep 12, 2024
@stuartwdouglas stuartwdouglas force-pushed the stuartwdouglas/active-fix branch 2 times, most recently from a8a8b33 to 9f95a9d Compare September 12, 2024 08:30
@@ -123,8 +123,8 @@ ORDER BY r.key;
SELECT sqlc.embed(d), m.name AS module_name, m.language, COUNT(r.id) AS replicas
FROM deployments d
JOIN modules m ON d.module_id = m.id
LEFT JOIN runners r ON d.id = r.deployment_id
WHERE min_replicas > 0 AND r.state = 'assigned'
LEFT JOIN runners r ON d.id = r.deployment_id AND r.state = 'assigned'
Copy link
Contributor

@jonathanj-square jonathanj-square Sep 12, 2024

Choose a reason for hiding this comment

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

nice pre-join filtering performance improvement!

Copy link
Collaborator Author

@stuartwdouglas stuartwdouglas Sep 12, 2024

Choose a reason for hiding this comment

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

It changes the query behaviour. If you have an 'idle' runner with the current query it will join the deployment, then get filtered out in the where clause. Filtering it here means that it just won't join, so the runner count will be zero but you still see the deployment row.

@stuartwdouglas stuartwdouglas force-pushed the stuartwdouglas/active-fix branch from 9f95a9d to 6d62bdf Compare September 12, 2024 23:21
@stuartwdouglas stuartwdouglas force-pushed the stuartwdouglas/active-fix branch from 6d62bdf to 181505f Compare September 12, 2024 23:50
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Sep 12, 2024
Merged via the queue into main with commit cf20910 Sep 13, 2024
89 checks passed
@stuartwdouglas stuartwdouglas deleted the stuartwdouglas/active-fix branch September 13, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants