lp:patchwork

Created by Jeremy Kerr on 2015-11-06 and last modified on 2020-03-19
Get this branch:
bzr branch lp:patchwork

Branch merges

Related bugs

Related blueprints

Branch information

Owner:
Patchwork Devs
Project:
Patchwork
Status:
Development

Import details

Import Status: Reviewed

This branch is an import of the HEAD branch of the Git repository at https://github.com/getpatchwork/patchwork.git.

The next import is scheduled to run in 3 hours.

Last successful import was 2 hours ago.

Import started 2 hours ago on izar and finished 2 hours ago taking 20 seconds — see the log
Import started 8 hours ago on alnitak and finished 8 hours ago taking 20 seconds — see the log
Import started 14 hours ago on izar and finished 14 hours ago taking 20 seconds — see the log
Import started 20 hours ago on izar and finished 20 hours ago taking 25 seconds — see the log
Import started on 2020-04-04 on alnitak and finished on 2020-04-04 taking 25 seconds — see the log
Import started on 2020-04-04 on izar and finished on 2020-04-04 taking 20 seconds — see the log
Import started on 2020-04-03 on alnitak and finished on 2020-04-03 taking 20 seconds — see the log
Import started on 2020-04-03 on izar and finished on 2020-04-03 taking 20 seconds — see the log
Import started on 2020-04-03 on izar and finished on 2020-04-03 taking 20 seconds — see the log
Import started on 2020-04-03 on alnitak and finished on 2020-04-03 taking 20 seconds — see the log

Recent revisions

1681. By Stephen Finucane <email address hidden> on 2020-03-19

trivial: Feed 'patchwork.migrations' through black

Include migrations in the flake8 checks, which should catch some simple
undefined variables errors.

Signed-off-by: Stephen Finucane <email address hidden>

1680. By Daniel Axtens on 2020-03-19

REST: Add release note for faster queries

Didn't quite seem like it fit anywhere else in the series. I want
the release note mostly because I hope to backport this to stable.

Signed-off-by: Daniel Axtens <email address hidden>
Reviewed-by: Stephen Finucane <email address hidden>

1679. By Daniel Axtens on 2020-03-19

REST: extend performance improvements to other parts of the API

We can trivially extend what we've just done to other parts of the API.

I haven't done much by way of benchmark but we're seeing multiple 'x's
pretty much across the board when filtering.

Signed-off-by: Daniel Axtens <email address hidden>
Reviewed-by: Stephen Finucane <email address hidden>

1678. By Daniel Axtens on 2020-03-19

REST: fix patch listing query

The patch listing query is punishingly slow under even very simple
filters.

The new data model in 3.0 will help _a lot_, so this is a simple fix: I
did try indexes but haven't got really deeply into the weeds of what we can do
with them.

Move a number of things from select_related to prefetch_related: we trade off
one big, inefficient query for a slightly larger number of significantly more
efficient queries.

On my laptop with 2 copies of the canonical kernel team list loaded into the
database, and considering only the API view (the JSON view is always faster)
with warm caches and considering the entire set of SQL queries:

 - /api/patches/?project=1
    ~1.4-1.5s -> <100ms, something like 14x better

 - /api/patches/?project=1&since=2010-11-01T00:00:00
    ~1.7-1.8s -> <560ms, something like 3x better (now dominated by the
                         counting query only invoked on the HTML API view,
                         not the pure JSON API view.)

The things I moved:

 * project: this was generating SQL that looked like:

   INNER JOIN `patchwork_project` T5
    ON (`patchwork_submission`.`project_id` = T5.`id`)

   This is correct but we've already had to join the patchwork_submission
   table and perhaps as a result it seems to be inefficient.

 * series__project: Likewise we've already had to join the series table,
   doing another join is possibly why it is inefficient.

 * delegate: I do not know why this was tanking performance. I think it
   might relate to the strategy mysql was using.

Reported-by: Konstantin Ryabitsev <email address hidden>
Signed-off-by: Daniel Axtens <email address hidden>
Reviewed-by: Stephen Finucane <email address hidden>

1677. By Daniel Axtens on 2020-03-19

REST: massively improve the patch counting query under filters

The DRF web view counts the patches as part of pagination.

The query it uses is a disaster zone:

  SELECT Count(*) FROM (
    SELECT DISTINCT
      `patchwork_submission`.`id` AS Col1,
      `patchwork_submission`.`msgid` AS Col2,
      `patchwork_submission`.`date` AS Col3,
      `patchwork_submission`.`submitter_id` AS Col4,
      `patchwork_submission`.`project_id` AS Col5,
      `patchwork_submission`.`name` AS Col6,
      `patchwork_patch`.`submission_ptr_id` AS Col7,
      `patchwork_patch`.`commit_ref` AS Col8,
      `patchwork_patch`.`pull_url` AS Col9,
      `patchwork_patch`.`delegate_id` AS Col10,
      `patchwork_patch`.`state_id` AS Col11,
      `patchwork_patch`.`archived` AS Col12,
      `patchwork_patch`.`hash` AS Col13,
      `patchwork_patch`.`patch_project_id` AS Col14,
      `patchwork_patch`.`series_id` AS Col15,
      `patchwork_patch`.`number` AS Col16,
      `patchwork_patch`.`related_id` AS Col17
    FROM `patchwork_patch`
    INNER JOIN `patchwork_submission`
      ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`)
    WHERE `patchwork_submission`.`project_id`=1
  )

This is because django-filters adds a DISTINCT qualifier on a
ModelMultiChoiceFilter by default. I guess it makes sense and they do a
decent job of justifying it, but it causes the count to be made with
this awful subquery. (The justification is that they don't know if you're
filtering on a to-many relationship, in which case there could be
duplicate values that need to be removed.)

While fixing that, we can also tell the filter to filter on patch_project
rather than submission's project, which allows us in some cases to avoid
the join entirely.

The resultant SQL is beautiful when filtering by project only:

  SELECT COUNT(*) AS `__count`
  FROM `patchwork_patch`
  WHERE `patchwork_patch`.`patch_project_id` = 1

On my test setup (2x canonical kernel mailing list in the db, warm cache,
my laptop) this query goes from >1s to ~10ms, a ~100x improvement.

If we filter by project and date the query is still nice, but still also
very slow:

  SELECT COUNT(*) AS `__count`
  FROM `patchwork_patch`
  INNER JOIN `patchwork_submission`
    ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`)
  WHERE (`patchwork_patch`.`patch_project_id`=1 AND `patchwork_submission`.`date`>='2010-11-01 00:00:00')

This us from ~1.3s to a bit under 400ms - still not ideal, but I'll take
the 3x improvement!

Reported-by: Konstantin Ryabitsev <email address hidden>
Signed-off-by: Daniel Axtens <email address hidden>
Reviewed-by: Stephen Finucane <email address hidden>

1676. By Stephen Finucane <email address hidden> on 2020-03-16

Patchwork v2.2.0-rc2

Signed-off-by: Stephen Finucane <email address hidden>
Signed-off-by: Daniel Axtens <email address hidden>

1675. By Mete Polat <email address hidden> on 2020-03-16

REST: Add patch relations

View relations and add/update/delete them as a maintainer. Maintainers
can only create relations of patches which are part of a project they
maintain. Because this is a writable many-many nested relationship, it
behaves a little unusually. In short:

- All operations use PATCH to the 'related' field of a patch

- To relate a patch to another patch, say 7 to 19, either:

    PATCH /api/patch/7 related := [19]
    PATCH /api/patch/19 related := [7]

- To delete a patch from a relation, say 1, 21 and 42 are related but we
  only want it to be 1 and 42:

    PATCH /api/patch/21 related := []

  * You _cannot_ remove a patch from a relation by patching another
    patch in the relation: I'm trying to avoid read-modify-write loops.

  * Relations that would be left with just 1 patch are deleted. This is
    only ensured in the API - the admin interface will let you do this.

- Break-before-make: if you have [1, 12, 24] and [7, 15, 42] and you want
  to end up with [1, 12, 15, 42], you have to remove 15 from the second
  relation first:

    PATCH /api/patch/1 related := [15] will fail with 409 Conflict.

  Instead do:

    PATCH /api/patch/15 related := []
    PATCH /api/patch/1 related := [15]
       -> 200 OK, [1, 12, 15, 42] and [7, 42] are the resulting relations

Signed-off-by: Mete Polat <email address hidden>
Signed-off-by: Stephen Finucane <email address hidden>
Signed-off-by: Daniel Axtens <email address hidden>

1674. By Mete Polat <email address hidden> on 2020-03-15

models, templates: Add patch relations

Introduces the ability to add relations between patches. Relations are
displayed in the details page of a patch under 'Related'. Related
patches located in another projects can be viewed as well.

Changes to relations are tracked in events. Currently the display of
this is very bare in the API but that will be fixed in a subsequent patch:
this is the minimum required to avoid throwing errors when you view the
events feed.

Signed-off-by: Mete Polat <email address hidden>
[dja: address some review comments from Stephen, add an admin view,
      move to using Events, misc tidy-ups.]
Signed-off-by: Daniel Axtens <email address hidden>

1673. By Stephen Finucane <email address hidden> on 2020-03-12

tests: Add tests for invalid cover/patch IDs

Signed-off-by: Stephen Finucane <email address hidden>
Related: #343

1672. By Andriy Gelman on 2020-03-11

views: Return Http404 if patch not found

Otherwise exception DoesNotExist shows error 500 on Apache

Signed-off-by: Andriy Gelman <email address hidden>
Signed-off-by: Stephen Finucane <email address hidden>
Closes: #343

Branch metadata

Branch format:
Branch format 7
Repository format:
Bazaar repository format 2a (needs bzr 1.16 or later)
This branch contains Public information 
Everyone can see this information.

Subscribers