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

SQL query audit + add database indexes #127

Open
tobscure opened this Issue Jun 20, 2015 · 9 comments

Comments

Projects
None yet
8 participants
@tobscure
Member

tobscure commented Jun 20, 2015

Lots of low-hanging fruit to improve performance and efficiency. Basically we just need to set up a query log, do every action possible on the forum, and then inspect each query individually and optimize where it's not too hard.

Also add SQL indexes (we don't have any currently apart from primary keys).

There are some more interesting cases too, like:

  • Flarum\Forum\Actions\IndexAction line 58: we send an API request to get the current user, but we already have the user data; we just want it formatted in an API response. We could technically call a serializer directly, but then we don't quite get all of the response features (like sideloaded data), and also Forum would depend on Api. I don't think we can optimize this case.
  • Flarum\Core\Repositories\EloquentPostRepository (see big comment at top of file)

@tobscure tobscure added the backend label Jun 20, 2015

@tobscure tobscure added this to the 1.0 Beta 1 milestone Jun 25, 2015

@tobscure tobscure changed the title from Audit SQL queries to SQL query audit + add database indexes Aug 27, 2015

@tobscure

This comment has been minimized.

Show comment
Hide comment
@tobscure

tobscure Aug 27, 2015

Member

No SQL query optimization has been done on Flarum yet, so there is probably quite a bit of room for performance gains here.

What needs to be done:

  • Create a "Debug" extension which attaches the page load time and a list of queries that were run to each API response (under the "meta" key).
  • Generate a large dataset (hundreds of thousands of discussions/posts) to test with.
  • Analyse the queries that are run on requests to each API endpoint and work out ways to reduce/optimize them – especially making sure there are no n+1 situations.
  • Add database indexes where appropriate.
Member

tobscure commented Aug 27, 2015

No SQL query optimization has been done on Flarum yet, so there is probably quite a bit of room for performance gains here.

What needs to be done:

  • Create a "Debug" extension which attaches the page load time and a list of queries that were run to each API response (under the "meta" key).
  • Generate a large dataset (hundreds of thousands of discussions/posts) to test with.
  • Analyse the queries that are run on requests to each API endpoint and work out ways to reduce/optimize them – especially making sure there are no n+1 situations.
  • Add database indexes where appropriate.

@tobscure tobscure removed this from the 1.0 Beta 1 milestone Aug 27, 2015

@tobscure tobscure referenced this issue Aug 28, 2015

Closed

v0.1.0 roadmap (old) #74

19 of 53 tasks complete

@tobscure tobscure added Performance Cleanup and removed backend labels Aug 28, 2015

@michaelmior

This comment has been minimized.

Show comment
Hide comment
@michaelmior

michaelmior Sep 1, 2015

I'm going to suggest https://github.com/maximebf/php-debugbar for this purpose as it's easy to use and can quickly collect a lot of helpful data.

I'm going to suggest https://github.com/maximebf/php-debugbar for this purpose as it's easy to use and can quickly collect a lot of helpful data.

@tobscure

This comment has been minimized.

Show comment
Hide comment
@tobscure

tobscure Sep 1, 2015

Member

Thanks, will check that out.

Member

tobscure commented Sep 1, 2015

Thanks, will check that out.

@justjavac justjavac referenced this issue Sep 7, 2015

Open

Flarum v0.1.0 开发路线图 #3

18 of 53 tasks complete

@tobscure tobscure added the Backend label Sep 16, 2015

@younes0

This comment has been minimized.

Show comment
Hide comment
@younes0

younes0 Sep 16, 2015

if it can help: Laravel's package of maximebf/debugbar: https://github.com/barryvdh/laravel-debugbar

younes0 commented Sep 16, 2015

if it can help: Laravel's package of maximebf/debugbar: https://github.com/barryvdh/laravel-debugbar

@franzliedke franzliedke modified the milestone: 0.1.0 Apr 7, 2016

@believer-ufa

This comment has been minimized.

Show comment
Hide comment

Hello! You can use Mysql profiling builted functions, for example.

@BartVB

This comment has been minimized.

Show comment
Hide comment
@BartVB

BartVB Jun 14, 2016

I would like to suggest another course of action. Analysing queries is fun stuff, but this mostly results in theoretical optimisation strategies. If you really want to improve performance it makes more sense to test with a database with thousands of users and millions of posts. This will make it much (much) easier to spot bottlenecks and you'll spot actual problems instead of indices which are missing only in theory.

Creating this database can be done with a fixtures script or by converting an existing large forum. Advantage of fixtures is that this is trivial to share with other developers and it's easier to adapt to the specs of your development environment. The main advantage of converting an existing forum is that this results in a very realistic database.

BartVB commented Jun 14, 2016

I would like to suggest another course of action. Analysing queries is fun stuff, but this mostly results in theoretical optimisation strategies. If you really want to improve performance it makes more sense to test with a database with thousands of users and millions of posts. This will make it much (much) easier to spot bottlenecks and you'll spot actual problems instead of indices which are missing only in theory.

Creating this database can be done with a fixtures script or by converting an existing large forum. Advantage of fixtures is that this is trivial to share with other developers and it's easier to adapt to the specs of your development environment. The main advantage of converting an existing forum is that this results in a very realistic database.

@franzliedke

This comment has been minimized.

Show comment
Hide comment
@franzliedke

franzliedke Jun 15, 2016

Member

@BartVB Good thinking - we can integrate this when we finally get to building the migration tool.

Still, there are some places where indices are clearly needed for basic filtering tasks, if I'm not mistaken...

Member

franzliedke commented Jun 15, 2016

@BartVB Good thinking - we can integrate this when we finally get to building the migration tool.

Still, there are some places where indices are clearly needed for basic filtering tasks, if I'm not mistaken...

@tobscure tobscure removed this from the 0.1.0 milestone Jul 22, 2017

@ganuonglachanh

This comment has been minimized.

Show comment
Hide comment
@ganuonglachanh

ganuonglachanh Aug 13, 2017

  1. As mentioned in the post : Slow mysql queries. The query:
    select `id` from `posts` where `type` in ('comment', 'discussionRenamed', 'discussionLocked', 'discussionStickied', 'discussionTagged') and `user_id` = '1' and `type` = 'comment' order by `time` desc limit 20 offset 0;
    need to optimize by index the colum posts.user_id
  2. In my forum, the post id 1 had been mentioned 33789 times. So this query will take 1-3 minutes to run.
    Flarum beta 0.7 will load all posts that mentioned this post to display. This should be limit by 200 or less.
    This should be fixed in
    https://github.com/flarum/flarum-ext-mentions/blob/master/src/Listener/AddPostMentionedByRelationship.php line 153
  1. As mentioned in the post : Slow mysql queries. The query:
    select `id` from `posts` where `type` in ('comment', 'discussionRenamed', 'discussionLocked', 'discussionStickied', 'discussionTagged') and `user_id` = '1' and `type` = 'comment' order by `time` desc limit 20 offset 0;
    need to optimize by index the colum posts.user_id
  2. In my forum, the post id 1 had been mentioned 33789 times. So this query will take 1-3 minutes to run.
    Flarum beta 0.7 will load all posts that mentioned this post to display. This should be limit by 200 or less.
    This should be fixed in
    https://github.com/flarum/flarum-ext-mentions/blob/master/src/Listener/AddPostMentionedByRelationship.php line 153

@tobscure tobscure added this to the v0.1.0-beta.8 milestone Sep 1, 2017

@tobscure tobscure self-assigned this Dec 19, 2017

@luceos luceos added Database and removed Backend labels Dec 19, 2017

@tobscure

This comment has been minimized.

Show comment
Hide comment
@tobscure

tobscure Mar 4, 2018

Member

@luceos List of database indexes to be added (probably in a separate PR from #1344, that one's already big enough) (sorry, have used the old column names):

core

users

  • join_time (sort user list)
  • last_seen_time (sort user list)
  • discussions_count (sort user list)
  • comments_count (sort user list)

discussions

  • last_time (sort discussion list)
  • last_user_id (filter discussion list)
  • start_time (sort discussion list)
  • start_user_id (filter discussion list)
  • comments_count (sort discussion list)
  • participants_count (sort discussion list)
  • hide_time (filter discussion list)

access_tokens

  • user_id (look up all of a user's tokens)

auth_tokens

  • created_at (garbage collection)

email_tokens

  • created_at (garbage collection)

notifications

  • user_id (look up + sort a user's notification list)

password_tokens

  • created_at (garbage collection)

posts

  • discussion_id + number (look up a post at a particular position within a discussion)
  • discussion_id + time (look up + sort a discussion's posts)
  • user_id + time (look up + sort a user's posts)

flarum-ext-lock

  • discussions: is_locked (filter discussion list)

flarum-ext-sticky

  • discussions: is_sticky + last_time (sort discussion list)

flarum-ext-flags

  • flags: post_id (retrieve flags for specific posts)
  • flags: time (sort global flag list)
Member

tobscure commented Mar 4, 2018

@luceos List of database indexes to be added (probably in a separate PR from #1344, that one's already big enough) (sorry, have used the old column names):

core

users

  • join_time (sort user list)
  • last_seen_time (sort user list)
  • discussions_count (sort user list)
  • comments_count (sort user list)

discussions

  • last_time (sort discussion list)
  • last_user_id (filter discussion list)
  • start_time (sort discussion list)
  • start_user_id (filter discussion list)
  • comments_count (sort discussion list)
  • participants_count (sort discussion list)
  • hide_time (filter discussion list)

access_tokens

  • user_id (look up all of a user's tokens)

auth_tokens

  • created_at (garbage collection)

email_tokens

  • created_at (garbage collection)

notifications

  • user_id (look up + sort a user's notification list)

password_tokens

  • created_at (garbage collection)

posts

  • discussion_id + number (look up a post at a particular position within a discussion)
  • discussion_id + time (look up + sort a discussion's posts)
  • user_id + time (look up + sort a user's posts)

flarum-ext-lock

  • discussions: is_locked (filter discussion list)

flarum-ext-sticky

  • discussions: is_sticky + last_time (sort discussion list)

flarum-ext-flags

  • flags: post_id (retrieve flags for specific posts)
  • flags: time (sort global flag list)

@luceos luceos self-assigned this Mar 5, 2018

@tobscure tobscure referenced this issue Apr 24, 2018

Open

Database naming changes #1236

7 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment