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

Use new rank view everywhere the old annotate_with_rank call was used. #509

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

n-rook
Copy link
Owner

@n-rook n-rook commented Aug 24, 2024

This should fix #507, as well as simplifying some code to no longer need rank_replays.py.

This should fix #507, as well as simplifying some code to no longer need
rank_replays.py.
Copy link
Collaborator

@jgdhs27 jgdhs27 left a comment

Choose a reason for hiding this comment

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

Awesome! What's the performance implication of this? I imagine it's a lot better now that you don't have to calculate it from scratch every time (idk how view tables work, are they stored in the DB?). The medal calculation was actually really slowing down how long it took to show replays on the home page and player pages.

@n-rook
Copy link
Owner Author

n-rook commented Aug 24, 2024

Unfortunately it's pretty much a wash for now. A view is basically just a query that the database pretends is a table. It didn't feel worse to me locally. I planned to click around on staging to see how it felt.

In the medium term what we should probably do is use a "materialized view" here:
https://www.postgresql.org/docs/current/rules-materializedviews.html

These are views that postgres saves to a table. The big caveat is that the table can then go stale and you have to go tell postgres whenever it should be updated. But this is a classic materialized view problem, since it's a small table with a big input and not that much information.

It'll be much easier to switch to a materialized view now that we have a regular view, so I figure this is still a step in the right direction. And it does fix the front page bug.

@n-rook n-rook merged commit 2df0eb3 into main Sep 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replays on recent uploads (that is, the front page) are ranked including TASes
2 participants