ThemeFuse / Unyson-Backups-Extension

Backup & Demo Content - This extension lets you create an automated backup schedule, import demo content or even create a demo content archive for migration purposes.
http://manual.unyson.io/en/latest/extension/backups/
10 stars 17 forks source link

Issues unzipping demo-content when wordpress root is on NFS #50

Closed cu12 closed 6 years ago

cu12 commented 7 years ago

I was testing an issue with a theme that uses your framework, where installing demo content was constantly failing.

After some investigation it turned out that it's most probably due to the reason we're using NFS as Wordpress root. It appears to be that due to async mounted NFS drive the OS reports the file has been sync'd, but in reality it hasn't been written on NFS yet.

My presumtion that this function could rather use sys_get_temp_dir in order to make it fail-proof.

andreiglingeanu commented 7 years ago

Hey @cu12,

Have you tried to swap out the implementation for get_tmp_dir()? Did it helped you?

cu12 commented 7 years ago

hey @andreiglingeanu

Indeed, dumbly modifying the function like this, makes this working

        public function get_tmp_dir() {
                return get_temp_dir() . '/tmp';
        }

it's a good idea to use the canonical function anyway.

andreiglingeanu commented 7 years ago

That's true, but I'm not entirely sure why that method was implemented that way in the first place. I'll have to look it up before swaping out the implementation.

cc @ViorelEremia

ViorelEremia commented 7 years ago

This function return path to folder fw-backup/tmp dir created in folder uploads this is not the same as default get_temp_dir() directly in tmp this is added and for another reasons it can be rewritten from config.php param dirs.destination, some plugins are badly programmed by deleting all of that folder it results that it will delete and your backup( plugins what works with downloaded files cache etc but not only plugins touch this folder). @cu12 can you check this function what path returns wp_upload_dir I think here is the problem.

cu12 commented 6 years ago

@ViorelEremia thanks for working on this, the function returns something like the following:

Array
(
    [path] => /var/www/wp-content/uploads/2017/11
    [url] => redacted
    [subdir] => /2017/11
    [basedir] => /var/www/wp-content/uploads
    [baseurl] => redacted
    [error] =>
)

Now the problem is not the path, but the filesystem behind it which is Gluster used via NFS. After the download finishes, it's not guaranteed that it's already been fsync'd to the filesystem, therefore an immediate read could end up in a "bad" zip file.

Making the download defaulting in /tmp would almost all the time guarantee that extraction would succeed right after the download as in 99% of the cases it's not a network filesystem.

ViorelEremia commented 6 years ago

I do not think I have to focus on this issue, and I do not know how to do this so as not to affect the others so if you find a solution create a pull request.