[03/11] Add index for patchwork_comment (submission_id,date)

Message ID 20180810080106.10714-4-stewart@linux.ibm.com
State Accepted
Headers show
Series
  • Performance for ALL THE THINGS!
Related show

Commit Message

Stewart Smith Aug. 10, 2018, 8 a.m.
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

Comments

Stephen Finucane Aug. 31, 2018, 2:08 p.m. | #1
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
Stewart Smith Sept. 10, 2018, 2:26 a.m. | #2
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.

Patch

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