reduce memory consumption of uploads and reindex task

[ewdurbin]

Resolves two memory consumption issues for projects with long/large release histories:

Uploads:

https://github.com/pypa/warehouse/blob/ce894db254d2f9097da784768f5e8b55252c169b/warehouse/forklift/legacy.py#L966-L970

Search indexing:

https://github.com/pypa/warehouse/blob/ce894db254d2f9097da784768f5e8b55252c169b/warehouse/packaging/search.py#L63-L79

[dstufft]

This looks to be redundant, sorting by Release._pypi_ordering should be the same as sorting by parse_version(r.version).

[dstufft]

Having load_only only a "generic" name like all_releases or latest_release makes me nervous because I feel like it's very easy to trigger N+1 query problems if this gets used in another context without realizing it, since the name doesn't imply anything about the fact it's limiting the data available.

What information do we actually use out of these? I don't remember what the templates do, but it looks like the search indexing really only cares about version numbers, is it the same for the templates? If it is maybe something like:

@property
def all_versions(self):
    return (orm.object_session(self)
               .query(Release.version)
               .filter(Release.project == self)
               .order_by(Release._pypi_ordering.desc())
               .all())

Would be enough to do the trick? We'd probably want a list() or something in there so the return type is just ["1.0", "2.0", 3.0"] in whatever order we expect this in.

Another thing to think about is making these just functions that aren't attached to the Project object, I'm not sure if that ends up being clearer or not.

[ewdurbin]

forklift/legacy.py also needs to pull in all_releases to re enumerate _pypi_ordering. I guess it'd be OK to do what's proposed here and inline the query for that.

[dstufft]

I believe that this is going to cause things that already use Project.all_releases to re-execute the all_releases query instead of re-using the already fetched values like it did previously. If we're going to end up executing a second query, we might as well handle all of the sorting in SQL. I also ask a similar question, is the only thing we really care about getting the latest version? then maybe this would be better off specified as:

@property
def latest_version(self):
    return (orm.object_session(self)
               .query(Release.version)
               .filter(Release.project = self)
               .order_by(
                   Release.is_prerelease.nullslast(),
                   Release._pypi_ordering.desc())
               .first())

[dstufft]

I forget why I did this, but is the latest version not already the one being passed into this method as release? If it is then this is redundant (and with these changes, causing an additional query to be executed).

[ewdurbin]

previously it skipped prereleases, so I kept that logic in place.

[dstufft]

Hmm, that raises a bit of a point, should we use the other data from a pre-release here? Is it confusing to have say the summary and readme from a pre-release being shown with the latest non pre-release version?

I think maybe the version selection logic for indexing should use the same nullslast, order by logic that I posted above that will give the first non-prerelease, unless there is only pre-releases.

[ewdurbin]

works for me!

[ewdurbin]

addressed.

addressed review.

[ewdurbin]

I think the biggest drawback to this implementation is the N+1 query situation introduced by calling all_versions against each Project.

Unfortunately I wasn't able to find a sane way to get stable memory consumption doing it all in one JOIN.

[ewdurbin]

Appears this may still not be working against the full database :/