-
Notifications
You must be signed in to change notification settings - Fork 944
feat(coderd/database): track user status changes over time #16019
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
Changes from 1 commit
cd953a3
ec16728
0d97e82
eb6e249
7b2c259
89177b2
877517f
0b3e0e6
6462cc2
ccd0cdf
12a274f
fcfd76e
7c0cade
ecffc8b
f3a2ce3
5067a63
254d436
3e86522
2e49e4c
ff59729
b1ad074
de4081f
4de334f
8fca0c5
b06179c
213b288
9457ac8
63128a3
89f0a11
89ebab2
012f14c
c2efd97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried about the number of CTEs and performance, but I think this is fine for now and let's not prematurely optimize. Just raising this so you're aware that in some cases a CTE performs worse than a pure query with joins and subqueries. That's mainly because the result of a CTE lacks indexes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about performance too, but user count and user status changes is going to be way less data than say workspace builds. Like on the order of 1000s. 🤷
Might be worth a benchmark, but we don't have a good way to populate "large" datasets atm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it this way for legibility. Trying to write the optimal solution here made my own query inscrutable even to myself. I don't consider this to be on a particularly hot path. We do have metrics for this query. If it needs to be optimised, we can do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also these CTEs were designed to reduce the amount of data being handled as early as possible. We use the existing indices while we have them to identify exactly the data we need ASAP and then once we need to join it all the hope is that we've brought down the volume low enough that it's performant regardless of the lack of indices on CTEs.