-
Notifications
You must be signed in to change notification settings - Fork 45
Speed up queries via materialized view #1671
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
base: main
Are you sure you want to change the base?
Conversation
Flix6x
left a comment
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.
Some early review comments. Nice to see the refresh logic taking shape.
Documentation build overview
Show files changed (4 files in total): 📝 4 modified | ➕ 0 added | ➖ 0 deleted
|
Flix6x
left a comment
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.
Exciting work! Thanks for addressing my earlier comments. Just a short code review for now, so you don't have to wait for me testing. Looking forward to testing tomorrow.
nhoening
left a comment
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 a quick review from me, before having tested myself, mostly about concepts.
I also suggest moving get_timed_belief_min_v() to timely-beliefs and add basic documentation there (not much more than the SQL command so anybody reading only that could in principle use it)
flexmeasures/data/migrations/versions/timed_beliefs_materialized_views.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Felix Claessen <[email protected]> Signed-off-by: Muhammad-Moiz626 <[email protected]>
| Performance optimizations | ||
| ---------------------------- | ||
|
|
||
| FLEXMEASURES_MVIEW_REFRESH_INTERVAL |
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.
This FLEXMEASURES_MVIEW_REFRESH_INTERVAL setting requires a short discussion, @Muhammad-Moiz626 @nhoening. I already scoped some options with @Ahmad-Wahid. I suggest to spin out that discussion on this comment thread.
There is currently no automation that runs the CLI command to refresh the mviews. Our go-to approach is to do this via a cronjob, for which we'd essentially have to move this setting in the cron job configuration. It doesn't then make sense to have that setting live in FlexMeasures, as FlexMeasures is not setting up cronjobs itself. I believe the config setting is, in this PR, only used to inform the user on the refresh rate in the UI. I think that is nice. If we want to keep that, then an alternative to this config setting may be to use the tooling we have that saves (in the db) the last time a specific CLI command was run successfully, and report that time in the UI instead.
Description
Our data model with multiple beliefs per event time (multiple horizons as well as sources) makes queries computationally expensive. Most-used are queries where we look only for the most recent beliefs about each event, when given any moment in time as max belief time.
Pre-computation with Postgres' materialized views can make queries much faster. This PR will enable hosts to use this performance enhancement.
...
Further Improvements
Related Items
Closes 108
...
Sign-off