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

Change scratch view pagination ordering to creation_time #952

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

ribbanya
Copy link
Contributor

@ribbanya ribbanya commented Feb 6, 2024

Recently, the site experienced massive slowdown, at around 11 AM UTC on February 5th, 2024. I believe that my usage of the API is what caused this.

I was using the /api/scratch endpoint to go very far back, filtering by SSBM scratches (preset=63). The last request I sent before the site stopped responding was:

https://decomp.me/api/scratch?cursor=cD0yMDIyLTA1LTA5KzA2JTNBMjclM0EyOC42NDYxMzYlMkIwMCUzQTAw&format=json&page_size=100&preset=63

This PR changes the ordering of the scratch view, as I believe that having a non-fixed ordering is forcing the site to re-index upon requesting records which have been updated since the last indexing. According to the documentation:

Proper usage of cursor pagination should have an ordering field that satisfies the following:

  • Should be an unchanging value, such as a timestamp, slug, or other field that is only set once, on creation.
  • Should be unique, or nearly unique. Millisecond precision timestamps are a good example. This implementation of cursor pagination uses a smart "position plus offset" style that allows it to properly support not-strictly-unique values as the ordering.
    [...]
  • The field should have a database index.

Additionally, @ethteck has noted in our conversation that "it's a bit more complicated, because we want the main page to still order by updates I think." But I don't have the experience to implement that, so I leave it up to you.

Copy link
Member

@ethteck ethteck left a comment

Choose a reason for hiding this comment

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

I take back what I said. This looks good to me!

Copy link
Member

@bates64 bates64 left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think the activity thing is a problem. Thanks for the detailed explanation

@ethteck ethteck merged commit 13968ef into decompme:main Feb 6, 2024
7 checks passed
@ribbanya ribbanya deleted the pr/scratch-view branch February 6, 2024 22:41
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.

None yet

3 participants