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

Albums: Add an index on album_filter to improve performance #1953

Closed
4 tasks
lastzero opened this issue Jan 18, 2022 · 6 comments
Closed
4 tasks

Albums: Add an index on album_filter to improve performance #1953

lastzero opened this issue Jan 18, 2022 · 6 comments
Assignees
Labels
enhancement Refactoring, improvement or maintenance task performance Performance Optimization released Available in the stable release

Comments

@lastzero
Copy link
Member

lastzero commented Jan 18, 2022

Adding an index on albums.album_filter can improve update performance:

Added PR #1804. For now only addressed the three UPDATE IN SELECT queries that popped up the most. Everything else seems more involved and might require changes to the DB scheme or at least other tricks, not sure how you feel about this at the current stage.

Regarding the DELETE query from my last comment, the performance issues get much better as soon as you add an index on album_filter to the albums table. For me it drops from 4.7s to 200ms without changing the query.

See GitHub Discussions:

The reason why there is/was no index is that there is a general length limit of 767 bytes for indexes in InnoDB tables that use the REDUNDANT or COMPACT row format.1

To display the current default row format:

SELECT @@innodb_default_row_format;

ToDo / Questions:

  • @srett: The key question now is if anyone could be using the REDUNDANT or COMPACT row format?
  • If yes, the only way is to put the filters in a separate table and index its primary key
  • Alternatively, we could limit the size of filters or add a album_filter_hash column and index that instead
  • Do SQLite and PostgreSQL have any (relevant) index size limitations? If yes: which?

Footnotes

  1. https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/innodb-limits.html

@lastzero lastzero self-assigned this Jan 18, 2022
@lastzero lastzero added enhancement Refactoring, improvement or maintenance task ux Impacts User Experience performance Performance Optimization and removed ux Impacts User Experience labels Jan 18, 2022
lastzero added a commit that referenced this issue Jan 18, 2022


Using a migration for testing on develop so nothing breaks (yet).
@lastzero
Copy link
Member Author

In case you wonder how much text fits in a VARBINARY(767) column:

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas suscipit ex vitae ex semper laoreet. Aliquam volutpat interdum odio, nec volutpat neque euismod id. Aliquam ac nibh at magna pellentesque blandit. Cras a mi rutrum, ultricies est ut, viverra tellus. Donec augue orci, euismod at justo ac, maximus eleifend velit. Etiam auctor nec ipsum vel condimentum. Nam at mattis nunc, a malesuada lorem. Aenean a ante pulvinar, imperdiet enim in, auctor ligula. Nullam luctus ac quam in sagittis. Ut varius libero arcu, vel hendrerit nisi varius at. Phasellus volutpat ante a nulla sodales, non rhoncus ante iaculis. Morbi libero mi, eleifend quis venenatis quis, pretium ac urna. Sed ullamcorper vulputate diam. Suspendisse pharetra, est eget dictum orci aliquam.

@lastzero lastzero added the please-test Ready for acceptance test label Jan 18, 2022
@lastzero
Copy link
Member Author

Merged the database migration for the albums table to the preview branch and started a Development Preview build, which should be available shortly.

Everyone is invited to test this on MariaDB and let us know if it helps!

lastzero added a commit that referenced this issue Jan 18, 2022
lastzero added a commit that referenced this issue Jan 18, 2022
@srett
Copy link
Contributor

srett commented Jan 19, 2022

I'd assume reducing the column length to 768 shouldn't hurt, 1024 wasn't that much more. Otherwise, a partial index might be worth a try. OTOH, I think even when PP was created, DYNAMIC was already the default row format, so at least all the docker-compose setups, even those that started out on very old versions, should be fine. Everyone else running their own custom setups can be expected to be competent enough to manually change the row format of their table before updating, if required.
Not so sure about SQLite and Postgres. For the latter it seems to be a close to 3kb for b-tree indexes, from a quick google search.

@lastzero
Copy link
Member Author

I remember we previously had problems with the limit, maybe it was a primary key.... although from my memory, it happened with file and/or path names, which is why we now use VARBINARY instead of VARCHAR for this.

@lastzero
Copy link
Member Author

@srett Does the change available in our Development Preview work for you / make a difference?

@srett
Copy link
Contributor

srett commented Jan 22, 2022

I see it already made it into the new release. Just upated and everything seems to be running fine; will import a batch of new photos later and see if anything explodes.

@lastzero lastzero added released Available in the stable release and removed please-test Ready for acceptance test labels Mar 6, 2022
@lastzero lastzero closed this as completed Mar 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Refactoring, improvement or maintenance task performance Performance Optimization released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

2 participants