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

Clear statcache before getting the mtime from local storage backends #10881

Merged
merged 2 commits into from
Dec 2, 2014

Conversation

icewind1991
Copy link
Contributor

@PVince81
Copy link
Contributor

PVince81 commented Sep 5, 2014

Will this have any side effects / performance impact ? I guess no since this is about the local storage.

@ghost
Copy link

ghost commented Sep 5, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7108/

@PVince81
Copy link
Contributor

PVince81 commented Sep 5, 2014

Did you try calling this in the text editor app instead and found out why it didn't work for the savefile.php ?

@@ -130,6 +130,7 @@ public function file_exists($path) {
}

public function filemtime($path) {
clearstatcache($path);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs the $this->datadir prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@icewind1991
Copy link
Contributor Author

@bantu fixed

@ghost
Copy link

ghost commented Sep 8, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7139/

@PVince81
Copy link
Contributor

PVince81 commented Sep 9, 2014

@icewind1991 see my question above: #10881 (comment)

@bantu
Copy link

bantu commented Sep 15, 2014

Will this have any side effects / performance impact ?

There is a cache miss involved for calls that look as follows now:

filemtime()
write()
filemtime()

I am not sure whether this is large problem. I would probably just go ahead and give this a try.

@bantu
Copy link

bantu commented Sep 15, 2014

Please note that the modification time may also be fetched by making a stat() call. Attribute 9, see http://php.net/manual/en/function.stat.php. So maybe the cache should also be cleared for stat() calls.

@PVince81
Copy link
Contributor

I'm ok with this 👍

@PVince81
Copy link
Contributor

@owncloud-bot retest this please

@ghost
Copy link

ghost commented Nov 26, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3318/
🚀 Test PASSed. 🚀

@scrutinizer-notifier
Copy link

The inspection completed: 2 new issues

@ghost
Copy link

ghost commented Dec 2, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3594/
🚀 Test PASSed. 🚀

@nickvergessen
Copy link
Contributor

Please also fix in MappedLocal.php

@icewind1991
Copy link
Contributor Author

Rebased, can this be merged?

@scrutinizer-notifier
Copy link

A new inspection was created.

@ghost
Copy link

ghost commented Dec 2, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3614/
🚀 Test PASSed. 🚀

@nickvergessen
Copy link
Contributor

👍

icewind1991 added a commit that referenced this pull request Dec 2, 2014
Clear statcache before getting the mtime from local storage backends
@icewind1991 icewind1991 merged commit 2bbb11f into master Dec 2, 2014
@icewind1991 icewind1991 deleted the touch-statcache branch December 2, 2014 22:29
@PVince81
Copy link
Contributor

@karlitschek @DeepDiver1975 @icewind1991 we need to backport this to stable7 to make the unit test of #14861 work (the rest works, only the unit tests requires that clearstatcache)

@karlitschek
Copy link
Contributor

backport is fine 👍

@lock lock bot locked as resolved and limited conversation to collaborators Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Owncloud does not serve the latest mtime of a file , it serves a cached mtime
7 participants