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

Cache submission annotated? as boolean #4379

Merged
merged 7 commits into from
Feb 3, 2023

Conversation

jorg-vr
Copy link
Contributor

@jorg-vr jorg-vr commented Feb 2, 2023

This pull request caches annotated? for every submission in the database. The goal is to avoid a lot of queries that check if a submission is annotated.

This query was third in our server time usage:
image
The cause was the check if a submission has annotations on every page were submissions are listed. (And for example on the activity page, this is loaded every time a user submits)

I also updated the released scope to also include left_joins(:evaluations) as this was a mandatory addition to get this scope working and thus always included when this scope was used.

I also used the new boolean to check whether annotations should be fetched on submission show.

I left the migration query in this time as it took only 52002.3ms on Naos and the impact is larger then in #4378 because we also use it to determine if annotations should be fetched.

  • Tests were added

replaces #4378

@jorg-vr jorg-vr self-assigned this Feb 2, 2023
@jorg-vr jorg-vr added the chore Repository/build/dependency maintenance label Feb 2, 2023
@jorg-vr jorg-vr temporarily deployed to naos February 2, 2023 13:50 — with GitHub Actions Inactive
@jorg-vr jorg-vr changed the title Chore/cache submission annotated as boolean Cache submission annotated? as boolean Feb 2, 2023
@jorg-vr jorg-vr marked this pull request as ready for review February 2, 2023 14:05
@jorg-vr jorg-vr requested a review from a team as a code owner February 2, 2023 14:05
@jorg-vr jorg-vr requested review from bmesuere and chvp and removed request for a team February 2, 2023 14:05
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

Looks good! I'm not sure about the migration query because a huge where clause with id's will be generated. I would iterate in batches over the annotations and update per batch. But if it works on naos, it will probably be fine.

@jorg-vr
Copy link
Contributor Author

jorg-vr commented Feb 3, 2023

Tested on naos. It does not generate a where clause with the ids. Rails is smart enough to generate a subquery, so everything happens in sql.

@jorg-vr jorg-vr merged commit 1d4c6d8 into develop Feb 3, 2023
@jorg-vr jorg-vr deleted the chore/cache-submission-annotated-as-boolean branch February 3, 2023 08:00
@bmesuere bmesuere added enhancement A change that isn't substantial enough to be called a feature and removed chore Repository/build/dependency maintenance labels Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A change that isn't substantial enough to be called a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants