| Message ID | 20180810080106.10714-4-stewart@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Performance for ALL THE THINGS! | expand |
On Fri, 2018-08-10 at 18:00 +1000, Stewart Smith wrote: > This (at least theoretically) should speed up displaying comments > on patches/cover letters. It's an index that will return rows > in-order for the query that we always do ("give me the comments > on this submission in date order"), rather than having to have > the database server do a sort for us. > > I haven't been able to benchmark something locally that shows > this is an actual improvement, but I don't have as large data > set as various production instances. The query plan does look > a bit nicer though. Although the benefit of index maintenance > versus how long it takes to sort things is a good question. Indexes scare me purely because there aren't clear "this is when you do this" and "then is when you don't do this" guidelines :) However, I trust your DB knowledge here and it should be easy to revisit this so Reviewed-by: Stephen Finucane <stephen@that.guru> Note that this migration will need to be renamed to 0028 and the Series.Meta modification hunk removed, owing to that belonging in patch 1. I've done this locally without issue. > Signed-off-by: Stewart Smith <stewart@linux.ibm.com> > --- > .../migrations/0027_add_comment_date_index.py | 23 +++++++++++++++++++ > patchwork/models.py | 4 ++++ > 2 files changed, 27 insertions(+) > create mode 100644 patchwork/migrations/0027_add_comment_date_index.py > > diff --git a/patchwork/migrations/0027_add_comment_date_index.py b/patchwork/migrations/0027_add_comment_date_index.py > new file mode 100644 > index 000000000000..0a57a9c3b212 > --- /dev/null > +++ b/patchwork/migrations/0027_add_comment_date_index.py > @@ -0,0 +1,23 @@ > +# -*- coding: utf-8 -*- > +# Generated by Django 1.11.15 on 2018-08-09 14:03 > +from __future__ import unicode_literals > + > +from django.db import migrations, models > + > + > +class Migration(migrations.Migration): > + > + dependencies = [ > + ('patchwork', '0026_add_user_bundles_backref'), > + ] > + > + operations = [ > + migrations.AlterModelOptions( > + name='series', > + options={'verbose_name_plural': 'Series'}, > + ), > + migrations.AddIndex( > + model_name='comment', > + index=models.Index(fields=['submission', 'date'], name='submission_date_idx'), > + ), > + ] > diff --git a/patchwork/models.py b/patchwork/models.py > index d2b88fc48c91..d2389cfdad29 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -625,6 +625,10 @@ class Comment(EmailMixin, models.Model): > class Meta: > ordering = ['date'] > unique_together = [('msgid', 'submission')] > + indexes = [ > + models.Index(name='submission_date_idx', > + fields=['submission','date']) > + ] > > > @python_2_unicode_compatible
Stephen Finucane <stephen@that.guru> writes: > On Fri, 2018-08-10 at 18:00 +1000, Stewart Smith wrote: >> This (at least theoretically) should speed up displaying comments >> on patches/cover letters. It's an index that will return rows >> in-order for the query that we always do ("give me the comments >> on this submission in date order"), rather than having to have >> the database server do a sort for us. >> >> I haven't been able to benchmark something locally that shows >> this is an actual improvement, but I don't have as large data >> set as various production instances. The query plan does look >> a bit nicer though. Although the benefit of index maintenance >> versus how long it takes to sort things is a good question. > > Indexes scare me purely because there aren't clear "this is when you do > this" and "then is when you don't do this" guidelines :) However, I > trust your DB knowledge here and it should be easy to revisit this so Hopefully we can get some index use statistics out of some big live sites and inform what we should keep / drop based on real data rather than gut feelings.
diff --git a/patchwork/migrations/0027_add_comment_date_index.py b/patchwork/migrations/0027_add_comment_date_index.py new file mode 100644 index 000000000000..0a57a9c3b212 --- /dev/null +++ b/patchwork/migrations/0027_add_comment_date_index.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.15 on 2018-08-09 14:03 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0026_add_user_bundles_backref'), + ] + + operations = [ + migrations.AlterModelOptions( + name='series', + options={'verbose_name_plural': 'Series'}, + ), + migrations.AddIndex( + model_name='comment', + index=models.Index(fields=['submission', 'date'], name='submission_date_idx'), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index d2b88fc48c91..d2389cfdad29 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -625,6 +625,10 @@ class Comment(EmailMixin, models.Model): class Meta: ordering = ['date'] unique_together = [('msgid', 'submission')] + indexes = [ + models.Index(name='submission_date_idx', + fields=['submission','date']) + ] @python_2_unicode_compatible
This (at least theoretically) should speed up displaying comments on patches/cover letters. It's an index that will return rows in-order for the query that we always do ("give me the comments on this submission in date order"), rather than having to have the database server do a sort for us. I haven't been able to benchmark something locally that shows this is an actual improvement, but I don't have as large data set as various production instances. The query plan does look a bit nicer though. Although the benefit of index maintenance versus how long it takes to sort things is a good question. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> --- .../migrations/0027_add_comment_date_index.py | 23 +++++++++++++++++++ patchwork/models.py | 4 ++++ 2 files changed, 27 insertions(+) create mode 100644 patchwork/migrations/0027_add_comment_date_index.py