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

Rename doesn't work inside unix volumes #1495

Closed
wants to merge 1 commit into from
Closed

Conversation

bbuie
Copy link

@bbuie bbuie commented Sep 17, 2016

but copy and unlink is identical and has no issue.

@malarzm
Copy link
Member

malarzm commented Sep 19, 2016

@bbuie thanks for the PR! What are the symptoms of not working rename though?

@bbuie
Copy link
Author

bbuie commented Sep 19, 2016

@malarzm Put the file your trying to rename in a volume. I'm using docker, so a
cache folder is in a folder on my local machine mounted to a folder in
Ubuntu 14.04. I get an error, "permission denied"

Here is more info: http://php.net/manual/en/function.rename.php#117590

On Monday, September 19, 2016, Maciej Malarz notifications@github.com
wrote:

@bbuie https://github.com/bbuie thanks for the PR! What are the
symptoms of not working rename though?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1495 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AC56Jh6FHYakRVuA_fypt_E-y33M9ADSks5qricugaJpZM4J_rPQ
.

Ben Buie
801.637.0437
ben@benbuie.com
Linkedin http://www.linkedin.com/in/benbuie | Twitter
https://twitter.com/benbuie | Facebook https://www.facebook.com/benbuie

@alcaeus
Copy link
Member

alcaeus commented Oct 5, 2016

This shouldn't be an issue. As you can see in the code, $tmpFileName is $fileName with a hash attached so the operation remains atomic. Assuming you're writing the cache files in /code/app/var/cache/doctrine/hydrators the rename would just be the following:

rename('/code/app/var/cache/doctrine/hydrators/SampleHydrator.php.abcdef', '/code/app/var/cache/doctrine/hydrators/SampleHydrator.php')

This rename shouldn't go across devices since the files stay in the same folder. Thus, I'm closing the issue - if you can reproduce this for us in a vagrant box feel free to open an issue providing tips how to reproduce it.

@alcaeus alcaeus closed this Oct 5, 2016
@bbuie
Copy link
Author

bbuie commented Oct 5, 2016

@alcaeus but it is an issue. In fact, it is a known issue to the php community:

http://php.net/manual/en/function.rename.php#117590

It happens when mongodb-odm is installed outside a volume and your trying to rename a file inside a volume.

Not only is the issue documented by the PHP community, it was an issue for me using mongodb-odm as part of a system built using docker.

It was a pain to find the fix and once found I patched it and submitted the changes to you (only two lines of changed code that is identical to the functionality of the old code) to make this code more accessible to other systems using volumes.

This patch fixed the issue on my machine and it will help others.

@alcaeus
Copy link
Member

alcaeus commented Oct 6, 2016

I have not yet managed to reproduce the problem, our docker machines don't show this problem at all, which is why I asked you to provide a way to reproduce the problem.

The reason why I'm hesitant to simply replace rename() with copy() followed by unlink() is atomicity: rename() is atomic while copy() isn't and can cause problems on slow file systems or with large files. You could end up with a script loading a partially written proxy file, causing a fatal error during script execution. That's why we write a temporary file and then rename it in the first place. If we were to do copy/unlink we might as well write the file directly.

@bbuie
Copy link
Author

bbuie commented Oct 6, 2016

@alcaeus that makes sense.

The only steps to recreate I can provide is setup a docker container where mongodb-odm is installed in the container but the file you're trying to rename is in a volume.

But I think I already said that, so I may need to table this.

@alcaeus
Copy link
Member

alcaeus commented Oct 11, 2016

I've tried reproducing this after running into a problem similar to this one and symfony/symfony#12533. So, I've come up with a gist that reproduces the general problem of cross-device renames: https://gist.github.com/alcaeus/a367e895e6c55f7fb93870dcba46efa9

The problem I was able to reproduce was renaming a folder across device, but I couldn't reproduce it with a file. Are you sure the problem you're hitting is due to ODM and not the Symfony cache?

@bbuie
Copy link
Author

bbuie commented Oct 11, 2016

@alcaeus I can't say for sure if it is with Symfony cache or ODM, I just know that by changing rename to copy and unlink it fixes my issue.

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

Successfully merging this pull request may close these issues.

None yet

3 participants