[PATCH v2 4/6] models: Migrate event fields to JSON field

[Stephen Finucane]

On Thu, 2018-04-12 at 11:50 +1000, Daniel Axtens wrote:
> Daniel Axtens <dja at axtens.net> writes:
> > Stephen Finucane <stephen at that.guru> writes:
> > > On Wed, 2018-04-11 at 02:23 +1000, Daniel Axtens wrote:

[snip]

> > > > > +def generate_payload(apps, schema_editor):
> > > > > +    # NOTE(stephenfin): We could convert this to native SQL in the future
> > > > > +    # by using e.g. 'JSON_OBJECT' for MySQL
> > > > > +    EventBase = apps.get_model('patchwork', 'Event')
> > > > > +
> > > > > +    for event in EventBase.objects.all():
> > > > > +        category = event.category
> > > > > +        # TODO(stephenfin): This logic should be moved into the EventSerializer
> > > > > +        # class
> > > > > +        if category == Event.CATEGORY_COVER_CREATED:
> > > > > +            payload_obj = Payload(cover=event.cover)
> > > > > +        elif category == Event.CATEGORY_PATCH_CREATED:
> > > > > +            payload_obj = Payload(patch=event.patch)
> > > > > +        elif category == Event.CATEGORY_PATCH_STATE_CHANGED:
> > > > > +            payload_obj = Payload(patch=event.patch,
> > > > > +                                  previous_state=event.previous_state,
> > > > > +                                  current_state=event.previous_state)
> > > > > +        elif category == Event.CATEGORY_PATCH_DELEGATED:
> > > > > +            payload_obj = Payload(patch=event.patch,
> > > > > +                                  previous_delegate=event.previous_delegate,
> > > > > +                                  current_delegate=event.current_delegate)
> > > > > +        elif category == Event.CATEGORY_PATCH_COMPLETED:
> > > > > +            payload_obj = Payload(patch=event.patch)
> > > > > +        elif category == Event.CATEGORY_CHECK_CREATED:
> > > > > +            payload_obj = Payload(patch=event.patch, check=event.created_check)
> > > > > +        elif category == Event.CATEGORY_SERIES_CREATED:
> > > > > +            payload_obj = Payload(series=event.series)
> > > > > +        elif category == Event.CATEGORY_SERIES_COMPLETED:
> > > > > +            payload_obj = Payload(series=event.series)
> > > > > +
> > > > > +        payload = PayloadSerializer(payload_obj).data
> > 
> > Converting this to json.dumps(PayloadSerializer(payload_obj).data)
> > WFM. It now 'only' uses about a third of my memory for 318k events.
> > 
> > The results seem to render although they are more nested than I remember:
> > 
> > {
> >         "id": 318637,
> >         "category": "patch-completed",
> >         "project": {
> >             "id": 2,
> >             "url": "http://localhost:8000/api/projects/2/",
> >             "name": "Kernel Team",
> >             "link_name": "kernel-team",
> >             "list_id": "kernel-team.lists.ubuntu.com",
> >             "list_email": "kernel-team at lists.ubuntu.com",
> >             "web_url": "",
> >             "scm_url": "",
> >             "webscm_url": ""
> >         },
> >         "date": "2018-04-10T15:24:19.650185",
> >         "payload": {
> >             "patch": {
> >                 "id": 105448,
> >                 "url": "http://localhost:8000/api/patches/105448/",
> >                 "msgid": "<20170828081014.24028-1-po-hsu.lin at canonical.com>",
> >                 "date": "2017-08-28T08:10:14",
> >                 "name": "[CVE-2016-10200,SRU,Trusty] l2tp: fix racy SOCK_ZAPPED flag check in l2tp_ip{, 6}_bind()",
> >                 "mbox": "http://localhost:8000/patch/105448/mbox/"
> >             }
> >         }
> >     },
> > 
> > ISTR they looked a bit different previously, although I can't remember
> > if the details (here things like mbox) were previous at the top level or
> > in some object.
> > 
> > Timings to come.
> 
> Preliminary results with 100 request to the old (non-refactored +
> prefetch_related) vs 100 requests to the new (this series):
> 
> Median time for old: 0.226s
>             for new: 0.298s
> 
> So this appears to be slower. I haven't done any rigorous testing or
> statistics though.
> 
> You can do better by changing select_related to prefetch_related. With
> that, I see a median of 0.197s. I don't think the complexity this
> patchset brings is justified by a ~30ms/req improvement though.

Totally agree. Wow, I guess the select_related -> prefetch_related and
JSON renderer changes made a bigger difference than I could have
imagined. I'm happy to drop most of this. Nice work!

The only aspect I would like to keep is a variant of the last two bits.
>From my brief testing, it seems like it's the filtering that's causing
the huge queries - not the JSON renderer itself. If we can disable this
from the web UI or, better again, switch to a AJAX-driven system, we'd
be able to speed up not only '/events' but everything else. Thoughts?

Stephen