Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cu12 opened this issue Sep 10, 2017 · 6 comments
Closed

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

cu12 opened this issue Sep 10, 2017 · 6 comments

Comments

@cu12
Copy link

cu12 commented Sep 10, 2017

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
Copy link
Collaborator

Hey @cu12,

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

@cu12
Copy link
Author

cu12 commented Sep 11, 2017

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
Copy link
Collaborator

andreiglingeanu commented Sep 11, 2017

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
Copy link
Contributor

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
Copy link
Author

cu12 commented Nov 7, 2017

@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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants