Move to subfolders for preview files #19214

rullzer · 2020-01-30T10:41:27Z

My take on #16747 @gbmaster please have a look as well.

Else the number of files can grow very large very quickly in the preview
folder. Esp on large systems.

This generates the md5 of the fileid. And then creates folders of the
first 7 charts. In that folder is then a folder with the fileid. And
inside there are the previews.

Since this is just an abstraction as to how things are stored. i chose to just extend appdata to handle it. This was non of the code touching it needs to be concerned with how things are stored. As long as they are stored (think of it as a poor mans objectstore).

kesselb · 2020-01-30T11:19:56Z

How do you delete previews of deleted files?
Another "most wanted" feature is to move previews somewhere else in the filesystem. Is that something you could implementation with the abstracted storage?

rullzer · 2020-01-30T11:33:28Z

1. How do you delete previews of deleted files?

Ah right I need to add the same logic to the background job to use the proper location.

2. Another "most wanted" feature is to move previews somewhere else in the filesystem. Is that something you could implementation with the abstracted storage?

Nope. Nextcloud expect data to be under the data directory.

kesselb · 2020-01-30T11:39:44Z

Ah right I need to add the same logic to the background job to use the proper location.

That's the tricky part 😞 Have fun 😃

gbmaster · 2020-01-30T12:18:48Z

Adding the logic to the background job is also where I stopped, as my Nextcloud DB schema knowledge was not enough to find a proper solution.
Feel free to use part of my commits, if you find them useful.

I am glad that you took over this task. Thank you very much!

rullzer · 2020-01-30T14:55:39Z

I think I have an idea... lets see ;)

rullzer · 2020-01-30T18:55:05Z

I think I have an idea... lets see ;)

seems to work... 27e0ded

lib/private/Preview/BackgroundCleanupJob.php

lib/private/Preview/Storage/Root.php

rullzer · 2020-02-07T07:32:56Z

Addressed. Please check and test

lib/private/Preview/Watcher.php

kesselb · 2020-02-07T09:51:13Z

Can we have a test for the old and new way in https://github.com/nextcloud/server/blob/master/tests/lib/Preview/BackgroundCleanupJobTest.php?

rullzer · 2020-04-02T18:55:02Z

All good I think
Please review. I'll squash once done :)

icewind1991 · 2020-04-03T15:42:35Z

Not quite

Test\Preview\BackgroundCleanupJobTest::testCleanupSystemCron
Doctrine\DBAL\Exception\DriverException: An exception occurred while executing 'SELECTDISTINCT "a"."name" FROM "oc_filecache" "a" LEFT JOIN "oc_filecache" "b" ON CAST("a"."name" AS INT) = "b"."fileid" LEFT JOIN "oc_filecache" "c" ON "a"."fileid" = "c"."parent" WHERE ("b"."fileid" IS NULL) AND ("a"."parent" = ?) AND ("a"."mimetype" = ?) AND ("c"."mimetype" <> ?)' with params [2709, 2, 2]:
SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "b"

skjnldsv

A few interesting comments, but code looks great so far! 🐘

rullzer · 2020-04-16T19:34:38Z

So now finally good I think.
Another review please

skjnldsv · 2020-04-16T19:35:51Z

Another review please

Still 👍 from me

kesselb · 2020-04-18T14:23:25Z

Lint / php-cs check (pull_request) Failing after 1m — php-cs check

Would you mind to run composer cs:fix?

Submodule 3rdparty updated 2540 files

🤔

tests/lib/Preview/BackgroundCleanupJobTest.php

kesselb · 2020-04-18T16:35:37Z

Seems to do the trick 👍 but a rebase is needed 😨

I don't like that after deleting a preview the folder structure stays even if there are no more previews 🤷‍♂️

rullzer · 2020-04-19T08:29:23Z

I don't like that after deleting a preview the folder structure stays even if there are no more previews man_shrugging

Yeah fair enough. I guess checking for that is rather straight forward to do.
BUt lets do that separately. As this has been idling long enough ;)

Else the number of files can grow very large very quickly in the preview folder. Esp on large systems. This generates the md5 of the fileid. And then creates folders of the first 7 charts. In that folder is then a folder with the fileid. And inside there are the previews. Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

kesselb · 2020-04-19T09:24:31Z

tests/lib/Preview/BackgroundCleanupJobTest.php

+		$this->assertSame(1, $this->countPreviews($root, $fileIds));
+		$job->run([]);
+
+		$root = $this->getRoot();


Commented that line locally and the test failed 🤔 Would you mind to explain why a new instance is needed?

…re to a subfolder structure * `php occ preview:repair` - a preview migration tool that moves existing previews into the new location introduced with #19214 * moves `appdata_INSTANCEID/previews/FILEID` to `appdata_INSTANCEID/previews/0/5/8/4/c/e/5/FILEID` * migration tool can be stopped during migration via `CTRL+C` - it then finishes the current folder (with the previews of one file) and stops gracefully * if a PHP memory limit is set in the `php.ini` then it will stop automatically once it has less than 25 MiB memory left (this is to avoid hard crashes in the middle of a migration) * the tool can be used during operation - possible drawbacks: * there is the chance of a race condition that a new preview is generated in the moment the folder is already migrated away - so the old folder with the newly cached preview is deleted and one cached preview needs to be re-generated * there is the chance of a race condition during access of a preview while it is migrated to the other folder - then no preview can be shown and results in a 404 (as of now this is an accepted risk) Signed-off-by: Morris Jobke <hey@morrisjobke.de>

MorrisJobke · 2020-08-06T19:38:56Z

A tool to migrate existing previews into this new format can be found at #22135.

…re to a subfolder structure * `php occ preview:repair` - a preview migration tool that moves existing previews into the new location introduced with #19214 * moves `appdata_INSTANCEID/previews/FILEID` to `appdata_INSTANCEID/previews/0/5/8/4/c/e/5/FILEID` * migration tool can be stopped during migration via `CTRL+C` - it then finishes the current folder (with the previews of one file) and stops gracefully * if a PHP memory limit is set in the `php.ini` then it will stop automatically once it has less than 25 MiB memory left (this is to avoid hard crashes in the middle of a migration) * the tool can be used during operation - possible drawbacks: * there is the chance of a race condition that a new preview is generated in the moment the folder is already migrated away - so the old folder with the newly cached preview is deleted and one cached preview needs to be re-generated * there is the chance of a race condition during access of a preview while it is migrated to the other folder - then no preview can be shown and results in a 404 (as of now this is an accepted risk) Signed-off-by: Morris Jobke <hey@morrisjobke.de>

rullzer added enhancement 2. developing Work in progress feature: previews and thumbnails labels Jan 30, 2020

rullzer added this to the Nextcloud 19 milestone Jan 30, 2020

rullzer requested review from icewind1991, ChristophWurst, juliushaertl, kesselb, tflidd and AasthaGupta January 30, 2020 10:41

nickvergessen reviewed Jan 31, 2020

View reviewed changes

lib/private/Preview/BackgroundCleanupJob.php Outdated Show resolved Hide resolved

kesselb reviewed Feb 2, 2020

View reviewed changes

lib/private/Preview/BackgroundCleanupJob.php Show resolved Hide resolved

kesselb reviewed Feb 2, 2020

View reviewed changes

lib/private/Preview/Storage/Root.php Outdated Show resolved Hide resolved

kesselb reviewed Feb 7, 2020

View reviewed changes

lib/private/Preview/Watcher.php Outdated Show resolved Hide resolved

rullzer force-pushed the enh/preview_folder_tree branch from fb9ef6e to 3afdfdd Compare March 2, 2020 15:00

rullzer force-pushed the enh/preview_folder_tree branch from 3afdfdd to 7304a23 Compare April 2, 2020 18:54

rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 2, 2020

rullzer mentioned this pull request Apr 4, 2020

19 beta 2 #20303

Merged

80 tasks

rullzer force-pushed the enh/preview_folder_tree branch from 7304a23 to 5284acf Compare April 8, 2020 13:36

skjnldsv approved these changes Apr 15, 2020

View reviewed changes

This was referenced Apr 15, 2020

19 beta 4 #20506

Merged

19 Beta 5 #20510

Merged

rullzer force-pushed the enh/preview_folder_tree branch from 353ed85 to 30ca622 Compare April 16, 2020 19:33

skjnldsv requested review from kesselb and nickvergessen April 16, 2020 19:35

kesselb reviewed Apr 18, 2020

View reviewed changes

tests/lib/Preview/BackgroundCleanupJobTest.php Show resolved Hide resolved

rullzer force-pushed the enh/preview_folder_tree branch from 30ca622 to 1b3cb1c Compare April 19, 2020 08:28

rullzer force-pushed the enh/preview_folder_tree branch from 1b3cb1c to 6c603e8 Compare April 19, 2020 08:31

skjnldsv requested a review from kesselb April 19, 2020 08:55

kesselb approved these changes Apr 19, 2020

View reviewed changes

rullzer merged commit 56a0700 into master Apr 19, 2020

rullzer deleted the enh/preview_folder_tree branch April 19, 2020 09:22

kesselb reviewed Apr 19, 2020

View reviewed changes

This was referenced Jul 29, 2020

Improving Filestructure of preview folder #14169

Closed

Improve distribution of previews and avatars to multiple object store buckets #22033

Closed

MorrisJobke mentioned this pull request Aug 6, 2020

Add occ preview:migrate to migrate previews from the old flat structure to a subfolder structure #22135

Merged

joshtrichards mentioned this pull request Jan 1, 2024

[Bug]: SQL errors generated by background job OC\\Preview\\BackgroundCleanupJob #42540

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to subfolders for preview files #19214

Move to subfolders for preview files #19214

rullzer commented Jan 30, 2020

kesselb commented Jan 30, 2020 •

edited

Loading

rullzer commented Jan 30, 2020

kesselb commented Jan 30, 2020

gbmaster commented Jan 30, 2020

rullzer commented Jan 30, 2020

rullzer commented Jan 30, 2020

rullzer commented Feb 7, 2020

kesselb commented Feb 7, 2020

rullzer commented Apr 2, 2020

icewind1991 commented Apr 3, 2020

skjnldsv left a comment

rullzer commented Apr 16, 2020

skjnldsv commented Apr 16, 2020

kesselb commented Apr 18, 2020

kesselb commented Apr 18, 2020

rullzer commented Apr 19, 2020

kesselb Apr 19, 2020

MorrisJobke commented Aug 6, 2020

Move to subfolders for preview files #19214

Move to subfolders for preview files #19214

Conversation

rullzer commented Jan 30, 2020

kesselb commented Jan 30, 2020 • edited Loading

rullzer commented Jan 30, 2020

kesselb commented Jan 30, 2020

gbmaster commented Jan 30, 2020

rullzer commented Jan 30, 2020

rullzer commented Jan 30, 2020

rullzer commented Feb 7, 2020

kesselb commented Feb 7, 2020

rullzer commented Apr 2, 2020

icewind1991 commented Apr 3, 2020

skjnldsv left a comment

Choose a reason for hiding this comment

rullzer commented Apr 16, 2020

skjnldsv commented Apr 16, 2020

kesselb commented Apr 18, 2020

kesselb commented Apr 18, 2020

rullzer commented Apr 19, 2020

kesselb Apr 19, 2020

Choose a reason for hiding this comment

MorrisJobke commented Aug 6, 2020

kesselb commented Jan 30, 2020 •

edited

Loading