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

db: improve belief search with new index #992

Merged
merged 6 commits into from
Mar 1, 2024
Merged

Conversation

nhoening
Copy link
Contributor

@nhoening nhoening commented Feb 28, 2024

Description

Loading beliefs takes too long. Next to a subquery improvement in timely-beliefs, we can also add an index on the 4 fields which usually are part of the queries. PR TB#167 makes sure a db migration catches the new index.

This PR includes this migration for FlexMeasures.

Look & Feel

flexmeasures db upgrade

How to test

In this PR I describe a small script to test the speedup (between before and after this migration).

I actually did not see a giant speedup, when the subquery optimization from TB#166 is already there. Without it, it can do a 50% improvement.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening requested a review from Flix6x February 28, 2024 14:37
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

To do: we'll need to set a new minimum timely-beliefs requirement after it's released.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
Copy link
Contributor Author

The more crucial reason to depend on a higher timely-beliefs version is SeitaBV/timely-beliefs#166 That one gets more bang for the buck.

We should merge both for a new version, if possible.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
Copy link
Contributor Author

nhoening commented Mar 1, 2024

@Flix6x Will you add the minimum timely-beliefs version in this PR when preparing a (dev/patch?) release?

Flix6x added 2 commits March 1, 2024 10:58
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added this to the 0.19.2 milestone Mar 1, 2024
@Flix6x Flix6x merged commit 9a5e739 into main Mar 1, 2024
7 checks passed
@Flix6x Flix6x deleted the db/add-beliefs-index branch March 1, 2024 10:05
Flix6x added a commit that referenced this pull request Mar 1, 2024
* make upgrading removing-obsolete-tables migration more stable

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* re-do migration, as we do not add the UNIQUE index after all

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* remove alembic comments

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* re-do migration based on changes in TB#167

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* chore: upgrade minimum requirement

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
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.

2 participants