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

Deleting an (external) storage mount leaves thumnails created in the filesystem #25033

Closed
mmattel opened this issue Jun 8, 2016 · 18 comments
Closed

Comments

@mmattel
Copy link
Contributor

mmattel commented Jun 8, 2016

Steps to reproduce

  1. Mount an (external) storage, in my case a SMB picture share
  2. Go into some folders via the files app - thumbs are created
  3. Delete the mount point

Expected behaviour

Automatic deletion of the corresponding thumbs which were related to this mountpoint

Actual behaviour

Thumbs from the deleted mountpoint are not deleted and stay in the users thumbnails directory as "orphans". Means, that this directory will grow forever, or you do a manual cleanup by deleting all as you do not know what belongs together.

Server configuration

Operating system: Ubuntu 14.04

Web server: nginx 1.9.14

Database: mysql

PHP version: 5.5.9

ownCloud version: (see ownCloud admin page)
ownCloud 9.1.0 beta 2 (git)

Updated from an older ownCloud or fresh install:
This was a fresh setup from tar and updated once, daily channel

Where did you install ownCloud from:
tar

Signing status (ownCloud 9.0 and above):

Datei nicht gefunden
Das ausgewählte Dokument wurde auf dem Server nicht gefunden.
Sie können zur Rückkehr zu ownCloud hier klicken.

List of activated apps:

Enabled:
  - activity: 2.3.2
  - comments: 0.3.0
  - dav: 0.2.4
  - federatedfilesharing: 0.3.0
  - federation: 0.1.0
  - files: 1.5.1
  - files_external: 0.6.0
  - files_sharing: 0.10.0
  - files_trashbin: 0.9.0
  - files_versions: 1.3.0
  - gallery: 15.0.0
  - logreader: true
  - provisioning_api: 0.5.0
  - systemtags: 0.3.0
  - updatenotification: 0.2.1
Disabled:
  - bookmarks
  - calendar
  - chat
  - contacts
  - documents
  - encryption
  - music
  - news
  - notes
  - testing
  - user_ldap

The content of config/config.php:

{
    "system": {
        "instanceid": "ock982ae3c5s",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "xxxx"
        ],
        "datadirectory": "\/var\/www\/owncloud\/data",
        "overwrite.cli.url": "https:\/\/xxxx",
        "dbtype": "mysql",
        "version": "9.1.0.8",
        "dbname": "owncloud",
        "dbhost": "localhost",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "logtimezone": "Europe\/Vienna",
        "installed": true,
        "filelocking.enabled": "true",
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "localhost",
            "port": 6379,
            "timeout": 0,
            "dbindex": 0
        },
        "assetdirectory": "\/var\/www\/owncloud",
        "asset-pipeline.enabled": true,
        "filesystem_check_changes": 1,
        "preview_max_scale_factor": 1,
        "enabledPreviewProviders": [
            "OC\\Preview\\PNG",
            "OC\\Preview\\JPEG",
            "OC\\Preview\\GIF",
            "OC\\Preview\\Illustrator",
            "OC\\Preview\\Postscript",
            "OC\\Preview\\Photoshop",
            "OC\\Preview\\TIFF",
            "OC\\Preview\\Raw"
        ],
        "loglevel": 2,
        "maintenance": false,
        "mail_smtpmode": "smtp",
        "mail_smtpsecure": "tls",
        "mail_from_address": "owncloud",
        "mail_domain": "xxxx",
        "mail_smtpauthtype": "LOGIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "xxxx",
        "mail_smtpport": "587",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***"
    }
}

Are you using external storage, if yes which one: local/smb/sftp/...
SMB

Are you using encryption: yes/no
No

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
No

Client configuration

Browser: Opera

Operating system: W7x64

Logs

owncloud.log

No corresponden log entry

@PVince81
Copy link
Contributor

PVince81 commented Jun 8, 2016

I believe there is no hook at storage deletion, so the previews subsystem doesn't know that it should delete them.

@georgehrke @oparoz

@PVince81 PVince81 added this to the 9.2-next milestone Jun 8, 2016
@rullzer
Copy link
Contributor

rullzer commented Jun 9, 2016

This should most likely be a background job. As I can imagine there being a whole lot of previews generated.

@oparoz
Copy link
Contributor

oparoz commented Jun 9, 2016

This should most likely be a background job

I agree

@mmattel
Copy link
Contributor Author

mmattel commented Jun 9, 2016

Could be. But how to remember / identify those previews that have belonged to a deleted mount point to be covered by the background job? Only those previews must be deleted, not all...

@PVince81
Copy link
Contributor

PVince81 commented Jun 9, 2016

I believe that the previews have the file id as file name. So one idea would be to go through all of them and check whether there is a matching entry in the oc_filecache table.

When deleting an external storage mount, in most cases the cache will be cleared for that storage.

@oparoz
Copy link
Contributor

oparoz commented Jun 9, 2016

The preview class also methods to delete previews, so maybe there doesn't need to talk to the DB directly.

@mmattel
Copy link
Contributor Author

mmattel commented Jun 9, 2016

Currently, if you delete a mount point, previews become orphants at the moment of deletion.
If you immediately run a files:scan command, these previews turn into "regular" files. So mount point deletion and related preview deletion must go hand in hand in one process.

@mmattel
Copy link
Contributor Author

mmattel commented Jun 10, 2016

@oparoz disagree, because we are not managing previews but the deletion of a mount point. This does not touch preview management and will not be triggered therefore.

Some thoughts:

For upcoming releases:

  • create a new oc_previews table containing path, storage ID of the mountpoint where they belong to (not the ID where they are written to = users home !) and etag equal to oc_filecache

oc_previews would need to be populated while generating previews as additional step, but has the advantage of not touching oc_filecache structure.

When deleting a mount point, you do the following:

  1. query oc_previews for the storage id to be removed (maybe in chunks because of quantity of results)
  2. loop thru the results and delete the files in the filesystem
  3. continue with removing the mount point (= not touching the current procedure)

The good thing is that nothing else changes in owncloud. Even if your system dies while running the deletion job and has not finished.

In general, the driver for preview file management is in this case not the filesystem (current) but owncloud as it has created those files and not the user!

For current releases we would need, if wanted, a fall back scenario. But the problem is distinguishing valid versus orphaned preview files.

@PVince81
Copy link
Contributor

or have the preview file list in "oc_filecache" for easier listing

@mmattel
Copy link
Contributor Author

mmattel commented Jun 10, 2016

Sometimes you need a break and new ideas come 😄
What about adding a column ref_storage in oc_filecache?
This would be completely open to other applications which may want/need to use it.
One drawback is, that you need to filter path component (thumbnails) with a text search for getting previews. But to speed up again, prefilter by ref_storage and the users home storage ID (fast) and filter the result by path component.

@PVince81
Copy link
Contributor

@mmattel can you expand your explanation ? What would be the value and use of "ref_storage" ?

@mmattel
Copy link
Contributor Author

mmattel commented Jun 10, 2016

Sure:

  • storage: this is the ID of the mount point where this file is physically located
  • ref_storage: this the ID of the mount point of the source where the data for the physical file was coming from

Example:

For previews:
target = function {source}

physical file location = target: location reference = storage
referenced location = source: location reference = ref_storage

oc_storages:
user_x: home_storage_location = ID:1
SMB: smb_share = ID:2

Examples for thumbnails for user_x:
Thumb_1: storage = 1 and ref_storage =1
Thumb_2: storage = 1 and ref_storage =2
ect...

If the user has more than one mounts, you will get possibly different ref_storage values for thumbnails. But this is exactly what we want 😄

On a generic view:
storage must be populated, ref_storage can be populated.

@PVince81
Copy link
Contributor

Ah, I see. This would be only for the thumbnail files.
If that is needed then I think I'd rather have the "oc_previews" table instead to avoid cluttering the "oc_filecache" table which is already huge on some systems.

@mmattel
Copy link
Contributor Author

mmattel commented Jun 10, 2016

The idea to have a ref_storage column is independent of where we put it to.

It is more a decision of if we do it for preview generation only, or be more generic and open.
Honestly thinking about this. I like the generic version more and more (whatever the future will bring...)

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically closed.

@stale stale bot closed this as completed Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants