Page MenuHomeSoftware Heritage
Differential D3976

loader: Stop materializing full lists of objects to be stored.
ClosedPublic
Actions

Authored by vlorentz on Sep 17 2020, 2:31 PM.

Details

Reviewers
olasd
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2373: git loader OOM when loading huge repository
Commits
rDLDBASE452fa224f9ca: loader: Stop materializing full lists of objects to be stored.
Summary

Since rDLDBASE43728c596498979cd5083b61e93360b4c2071c31, store_data consumes the entire iterator
of contents, and since 3b97703d7f14e145d6124f1c61f5f283ee8eecf2, it does the same for
other object types.

This causes all the (new) objects of the loaded repository to be loaded
in memory at the same time before being sent to the storage, which can
cause OOM errors.

Instead, with this commit, objects are added one by one to the storage,
which restores the lazy behavior we had before these two commits using
the buffered storage proxy.

Resolves T2373.

Diff Detail

Event Timeline

vlorentz created this revision.Sep 17 2020, 2:31 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 17 2020, 2:31 PM
vlorentz edited the summary of this revision. (Show Details)Sep 17 2020, 2:32 PM
vlorentz added a task: T2373: git loader OOM when loading huge repository.
swh-public-ci added a comment.Sep 17 2020, 2:33 PM

Build is green

Patch application report for D3976 (id=14010)

Rebasing onto fbe906c0c9...

First, rewinding head to replay your work on top of it...
Applying: loader: Stop materializing full lists of objects to be stored.
Changes applied before test
commit dbf4271aad183f96a22843dc7c5ae84ce5f72ce9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Sep 17 14:31:22 2020 +0200

    loader: Stop materializing full lists of objects to be stored.
    
    Since 43728c596498979cd5083b61e93360b4c2071c31, store_data consumes the entire iterator
    of contents, and since 3b97703d7f14e145d6124f1c61f5f283ee8eecf2, it does the same for
    other object types.
    
    This causes all the (new) objects of the loaded repository to be loaded
    in memory at the same time before being sent to the storage, which can
    cause OOM errors.
    
    Instead, with this commit, objects are added one by one to the storage,
    which restores the lazy behavior we had before these two commits using
    the buffered storage proxy.

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/294/ for more details.

Harbormaster completed remote builds in B15237: Diff 14010.Sep 17 2020, 2:33 PM
olasd added a subscriber: olasd.Sep 17 2020, 2:41 PM

I really hate this, but save from making *_add able to take generators as arguments again, this is probably the most decent way out.

vlorentz mentioned this in D3978: tests: Don't check the number of created 'person' objects..Sep 17 2020, 2:51 PM
olasd accepted this revision as: olasd.Sep 17 2020, 4:00 PM

Begrudgingly giving this an Accept.

This definitely needs a comment in the code, because I'm sure the next person reading this code will be very confused.

This revision is now accepted and ready to land.Sep 17 2020, 4:00 PM
ardumont accepted this revision as: ardumont.Sep 17 2020, 4:03 PM
ardumont edited the summary of this revision. (Show Details)Sep 17 2020, 4:07 PM
ardumont accepted this revision.Sep 17 2020, 6:11 PM
vlorentz updated this revision to Diff 14028.Sep 17 2020, 6:16 PM

rebase

vlorentz updated this revision to Diff 14029.Sep 17 2020, 6:16 PM

rebase

This revision was landed with ongoing or failed builds.Sep 17 2020, 6:16 PM
Closed by commit rDLDBASE452fa224f9ca: loader: Stop materializing full lists of objects to be stored. (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDLDBASE452fa224f9ca: loader: Stop materializing full lists of objects to be stored..
swh-public-ci added a comment.Sep 17 2020, 6:20 PM

Build is green

Patch application report for D3976 (id=14028)

Rebasing onto 452fa224f9...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-297-D3976.
Changes applied before test



See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/297/ for more details.
Harbormaster completed remote builds in B15297: Diff 14028.Sep 17 2020, 6:20 PM
swh-public-ci added a comment.Sep 17 2020, 6:22 PM
Build is green



Patch application report for D3976 (id=14029)



Rebasing onto 452fa224f9...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-298-D3976.





Changes applied before test






See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/298/ for more details.
Harbormaster completed remote builds in B15298: Diff 14029.Sep 17 2020, 6:22 PM
ardumont mentioned this in D3986: loaders: Move the proxy storage filter after the buffer proxy.Sep 18 2020, 2:05 PM
ardumont mentioned this in D3988: loaders: Move the proxy storage filter after the buffer proxy.Sep 18 2020, 2:44 PM
ardumont mentioned this in T2373: git loader OOM when loading huge repository.Sep 18 2020, 2:57 PM
ardumont mentioned this in rSPSITE76db1fae5c5b: loaders: Move the proxy storage filter after the buffer proxy.Sep 18 2020, 3:12 PM
ardumont mentioned this in rDENV5870fc161457: loaders: Move the proxy storage filter after the buffer proxy.Sep 18 2020, 3:17 PM
Revision Contents
Changeset List
PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
core/
17 lines79%
Loading...
Diff 14010
View Options
swh/loader/core/loader.py
Loading...
About · Developer information · Legal