Fix n+1 query problem with event.series.title
#1314
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
At some point, videolist blocks started requesting the events series title (which in case of the playlist block, is shown in list view). But that causes one query per event in each video block. This is a problem in particular for pages with lots of videolist blocks with lots of videos. The solution for this is that each time an event is loaded from DB, the series title and opencast ID is also loaded. If its not used, it's just a tiny waste of DB performance, but if the GraphQL request requests it, we can immediately use it. This uses the look-ahead feature of Juniper. I'm not a huge fan of this solution as it only fixes this one thing and does not present a general solution. However, a general solution for "execute the fewest and smartest DB queries for a GQL request" is super hard if not impossible. So I still think this should be merged, even if it adds complexity for now. I disabled the `impl_from_db` block and went through all error messages to find places where events are loaded from the DB. I am pretty sure that's everything I needed to change.
Draft
owi92
approved these changes
Jan 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At some point, videolist blocks started requesting the events series title (which in case of the playlist block, is shown in list view). But that causes one query per event in each video block. This is a problem in particular for pages with lots of videolist blocks with lots of videos.
The solution for this is that each time an event is loaded from DB, the series title and opencast ID is also loaded. If its not used, it's just a tiny waste of DB performance, but if the GraphQL request requests it, we can immediately use it. This uses the look-ahead feature of Juniper.
I'm not a huge fan of this solution as it only fixes this one thing and does not present a general solution. However, a general solution for "execute the fewest and smartest DB queries for a GQL request" is super hard if not impossible. So I still think this should be merged, even if it adds complexity for now.
I disabled the
impl_from_dbblock and went through all error messages to find places where events are loaded from the DB. I am pretty sure that's everything I needed to change.