-
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
[POC] Store file id in extended attributes where possible #14546
Conversation
@@ -213,12 +213,27 @@ protected function removeFromCache($path) { | |||
protected function addToCache($path, $data, $fileId = -1) { | |||
\OC_Hook::emit('Scanner', 'addToCache', array('file' => $path, 'data' => $data)); | |||
$this->emit('\OC\Files\Cache\Scanner', 'addToCache', array($path, $this->storageId, $data)); | |||
if ($this->storage->instanceOfStorage('OCP\Files\Storage\IAttribute') and $fileId === -1) { |
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.
Does it mean you need to add getAttribute
and setAttribute
to the default storage wrapper, to make sure they are forwarded to the underlying storage ?
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.
No, there are magic methods to handle that
Sweet. We should probably also check for file id clashes. What happens if two files have the same file id attribute ? (for whatever reason) |
The inspection completed: No issues found |
Refer to this link for build results (access rights to CI server needed): |
@icewind1991 rebase needed |
I wonder if we should do this as an optional separate app and register a storage wrapper or something ? This requires the FS to support xattr and the FS must be local, so no external storage (except the one of type "Local"). |
Some external storages support adding arbitrary metadata to files which could be used. I'm not sure if this can be cleanly implemented in an app due to the changes to the scanner logic (which apps cant overwrite without hacks) |
Would it make sense to move this to an app instead ? |
This could be an alternative implementation for the upcoming storage based cache Händler.i vote die close for the time being. |
Requires http://pecl.php.net/package/xattr to be installed and the filesystem to support user extended attributes (default on btrfs, mount option for ext(3|4) iirc)
To test:
cc @PVince81 @DeepDiver1975