os.system() forks duplicity, doubles memory usage

Bug #1684312 reported by Wade Rossmann on 2017-04-19
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Duplicity
Medium
Unassigned

Bug Description

It took a bit of a journey to arrive at this particular bug, so please bear with me on the following sad story.

1. Backups kept dying because `duplicity-${mode}.${timestamp}.manifest.part` was not being renamed to `duplicity-${mode}.${timestamp}.manifest`
2. Went completely down the rabbit hole and found via strace that immediately before the failure there's a failed `clone()` call with `1 ENOMEM (Cannot allocate memory)`. [note: we have fat backups so *one* duplicity process uses most of the memory, and doubling that is just out of the question]
3. Dug into the source and found that there's literally *one* `os.fork()` call, and that's in duplicity/gpginterface.py:414. [this was a dead end, but forking is going to be problematic here as well]
4. Spun up a duplicity backup with strace on a non-broken server and watched for what the fork actually *did*. Turns out it does `execve('sh', '-c', 'cp ...')` and are you flippin kidding me?
5. Checked in with #python and confirmed that os.system() causes a fork and got quite a bit of sass on your behalf for using it at all.
6. Monkey-patched production, and lo and behold the backup completes!

= Reproduction:

Run a backup with `--no-encryption`.

Gzip and GPG modes don't invoke this particular flavor of problem, though the aforementioned fork in gpginterface.py likely isn't doing you guys any favors.

= Fix:

Replace duplicity/dup_temp.py:187

    os.system("cp -p \"%s\" \"%s\"" % (src.name, tgt.name))

With:

    from shutil import copyfile
    copyfile(src.name, tgt.name)

That's literally it and you've halved your memory footprint at the end of the run.

= Errata:
- duplicity versions: 0.7.10 and 0.7.12
- OS: Centos 7.2
- FS: ext4

Changed in duplicity:
importance: Undecided → Medium
milestone: none → 0.7.13
status: New → Fix Committed
Changed in duplicity:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers