Merge lp://staging/~cjwatson/launchpad/limit-debdiff into lp://staging/launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 17862
Proposed branch: lp://staging/~cjwatson/launchpad/limit-debdiff
Merge into: lp://staging/launchpad
Diff against target: 288 lines (+118/-45)
5 files modified
lib/lp/services/config/schema-lazr.conf (+16/-0)
lib/lp/soyuz/doc/package-diff.txt (+0/-27)
lib/lp/soyuz/model/packagediff.py (+31/-2)
lib/lp/soyuz/model/sourcepackagerelease.py (+2/-12)
lib/lp/soyuz/tests/test_packagediff.py (+69/-4)
To merge this branch: bzr merge lp://staging/~cjwatson/launchpad/limit-debdiff
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+278187@code.staging.launchpad.net

Commit message

Kill debdiff after ten minutes or 1GiB of output by default, and make sure we clean up after it properly. Add a configurable blacklist.

Description of the change

Kill debdiff after ten minutes or 1GiB of output by default, and make sure we clean up after it properly. Add a configurable blacklist.

Some source packages that contain particularly convoluted symlink farms can confuse debdiff into producing exponentially large output, and we should guard ourselves against this possibility. Ten minutes seems to be a reasonable threshold, as it's larger than the time taken for 99.9% of all successful PackageDiffJobs in 2015 to complete, but I've made it configurable in case we need to tweak it in future. I've arranged to set TMPDIR because debdiff creates some of its own temporary files there and may not clean them up properly if it's killed.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)
Revision history for this message
Robert Collins (lifeless) wrote :

It might be a good idea to set a direct metric on the output work too - e.g. set a ulimit on memory, total cpu or when more than some N (e.g. 1M) of output is generated stop immediately.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.