[PATCH v2 0/9] Add support for Django 1.11

Stephen Finucane stephen at that.guru
Mon Dec 4 09:55:53 AEDT 2017


On Mon, 2017-12-04 at 09:29 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen at that.guru> writes:
> 
> > On Thu, 2017-11-16 at 21:02 +1100, Daniel Axtens wrote:

[snip]

> > > I haven't specifically examined the patches to the level I'm
> > > comfortable with giving them Reviews, but I am confident that
> > > they work.
> > > 
> > > Earlier you talked about a performance regression with 1.11 - did
> > > you ever find out anything about that? I have been looking at
> > > doing a proper performance test suite, but haven't been able to
> > > carve out a chunk of time to do it...
> > 
> > Aye, this was the issue I pointed out with the '/api/events'
> > endpoint. As you've noted, this is unrelated to Django 1.11 and has
> > to be resolved separately (I'm working on it).
> 
> I have made some (so far incomplete) attempts centering around
> denormalising the project field - moving it either completely out of
> submission and into patch/coverletter/comment or just duplicating it
> in those three models. This also fixes the fact that we need a SQL
> join just to enumerate patches which is slow (and an OzLabs pain
> point) - but the migrations are a paaaaaiiiiiin. Is that your
> approach or do you have other ideas?

Not quite. I've been focused on the 'Event' model mostly and getting
rid of the most of the ForeignKeys contained therein. To do this, I'm
replacing the cover letter and patch foreign keys with a submission
reference and storing a serialized JSON blob for things like
'previous_delegate' (the payload) instead. This tightly binds us to the
REST API representation and means we lose things the integrity
constraints that foreign keys provide. However, we only expose this
stuff over that API and we don't generally delete anything so the
constraints don't give us much. As for migrations, I've got a feature
to automatically delete ("expire") events after an interval (defaulting
to 30 days). The main point of this is that events lose value the older
they are, but I figure if this run before the migration, it would limit
the amount of things we have to migrate.

I haven't yet looked at removing the join for patches and cover
letters, but have you thought of simply folding these in and using an
enum-like field to indicate if it's a cover letter or patch? The
migration for that is also going to be tough and it's going to
denormalize things, but if the JOINs are a serious issue it might be
the only option we have.

Stephen


More information about the Patchwork mailing list