-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Dont update the cache when working with part files #13836
Conversation
Nice. How about the other cache operations like delete ? (which might happen when renaming a part file to final file) |
1215c9b
to
fd68e85
Compare
fd68e85
to
adf9a4e
Compare
The inspection completed: 3 new issues, 1 updated code elements |
Refer to this link for build results (access rights to CI server needed): |
@@ -811,7 +811,7 @@ private function basicOperation($operation, $path, $hooks = array(), $extraParam | |||
if (in_array('delete', $hooks) and $result) { | |||
$this->updater->remove($path); | |||
} | |||
if (in_array('write', $hooks)) { | |||
if (in_array('write', $hooks) and $operation !== 'fopen') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have more consequences, but possibly positive.
I remember seeing it update the file cache with size 0 whenever a file was getting opened. Didn't make much sense, so this change might help. I hope it won't break other things though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify: this was in the cross-storage rename case where files were copied to the target storage. Since they used fopen() it would create an empty file first in the cache.
👍 works Moving files cross storage also still works as before. (it causes a redownload, but that is another issue that exists on master: #13310) |
Needs a second reviewer @DeepDiver1975 @nickvergessen @MorrisJobke @schiesbn |
👍 |
Dont update the cache when working with part files
The actually write to the cache is already not done for the part files but atm it still trigers things like the change propagator.
Also includes a fix to not update the cache after an fopen (since the file isn't changed until the handle is actually used)
Combined it saves 14 queries when doing a WebDAV PUT in my instance (comparison)
cc @DeepDiver1975 @PVince81