Moodle
  1. Moodle
  2. MDL-8605

Reduce the number of dirs under the users file area

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.8, 1.9
    • Fix Version/s: 1.8.4, 1.9, 2.0
    • Component/s: Administration
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      29126

      Description

      Currently (all Moodle versions) under the moodledata/users directory, one dir with the userid is created to store, basically, the avatar of the user. This system has two big drawbacks:

      1) BIG sites are beginning to raise the "Max files per dir" OS limit. And usually, this is one hard limit.
      2) The structure under each userid directory is pretty plain and we should start to accommodate it for future personal storage.

      So, I would propose to:

      1) Create one simple algorithm that, based in one number (userid), will return one unique hash of fixed length.
      2) Create one function - create_user_dir($userid) - in order to create the user storage area. Such storage area should be created by creating nested subdirs with parts of the hash calculated in step 1 (nor raising, say, 12 bits, per name = 4096 dirs per dir). This function could have one optional parameter ($migrate = false), to allow migration from old user storage areas to the new ones transparently.
      3) Create one function - get_user_dir($userid) - in order to get the path to the user storage area. This function should check for the new areas + the current ones in order to keep compatibility.
      4) Inside each user storage area, create one "private" dir, where we'll move the avatars. Such "private" dir is intended to store files that are handled by Moodle but without FileManager access for students in the future.
      5) Change all current uses of user storage area to use the functions defined above.
      6) Document it to allow developers to know how they must handle files in that zone.

      Sounds simple. Ciao

      1. hash.php
        0.8 kB
        Eloy Lafuente (stronk7)
      2. newhash.php
        0.8 kB
        Iñaki Arenaza

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oops, I hope this should be only for 1.9 (if approved). Deleting 1.8 as target.

          Show
          Eloy Lafuente (stronk7) added a comment - Oops, I hope this should be only for 1.9 (if approved). Deleting 1.8 as target.
          Hide
          Jose Rama added a comment -

          hash is not-only by definition, because not a function type CRC?. In addition, to adhere to a standard one of filesystem, one quite restrictive one, as it can be the ISO9660.

          Show
          Jose Rama added a comment - hash is not-only by definition, because not a function type CRC?. In addition, to adhere to a standard one of filesystem, one quite restrictive one, as it can be the ISO9660.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh, yep. I the "hash" name isn't the correct one although I talked about "unique hash". I've attached one simple example of calculating user storage area based one userid.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oh, yep. I the "hash" name isn't the correct one although I talked about "unique hash". I've attached one simple example of calculating user storage area based one userid. Ciao
          Hide
          Jose Rama added a comment -

          for example :
          ID= 123 hash_fix_length=6 userid_to_path(ID=123)-> /0/0/0/1/2/3/
          ID= 0 ( admin ) hash_fix_length=6 userid_to_path(ID=0)-> /0/0/0/0/0/0/
          ID=999999 hash_fix_length=6 userid_to_path(ID=0)-> /9/9/9/9/9/9/

          one millon users, or also we can use hexadecimal notation for ID
          and we needed a tree with less levels ( hash_fix_length=5 for 1 millon users)

          I see if I can make a draft of the other points

          Show
          Jose Rama added a comment - for example : ID= 123 hash_fix_length=6 userid_to_path(ID=123)-> /0/0/0/1/2/3/ ID= 0 ( admin ) hash_fix_length=6 userid_to_path(ID=0)-> /0/0/0/0/0/0/ ID=999999 hash_fix_length=6 userid_to_path(ID=0)-> /9/9/9/9/9/9/ one millon users, or also we can use hexadecimal notation for ID and we needed a tree with less levels ( hash_fix_length=5 for 1 millon users) I see if I can make a draft of the other points
          Hide
          Martin Dougiamas added a comment -

          Is this something we should consider moving to the database?

          Show
          Martin Dougiamas added a comment - Is this something we should consider moving to the database?
          Hide
          Martin Dougiamas added a comment -

          Profile images I mean... this would still be useful for user files, sure, in the new local repository.

          Show
          Martin Dougiamas added a comment - Profile images I mean... this would still be useful for user files, sure, in the new local repository.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I really think that it's better to have all the user files stored in FS for now (it would be easy to make avatars invisible for users once the local repository is working).

          Then, someday, we should be able to abstract all the current file system accesses and create, in a pluggable fashion, all sort of storing systems (file system, local or external DB, other CMS...over mnet...).

          That's my point of view. Although it would be easy to store them in DB, just analysing if it's the moment to do that.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I really think that it's better to have all the user files stored in FS for now (it would be easy to make avatars invisible for users once the local repository is working). Then, someday, we should be able to abstract all the current file system accesses and create, in a pluggable fashion, all sort of storing systems (file system, local or external DB, other CMS...over mnet...). That's my point of view. Although it would be easy to store them in DB, just analysing if it's the moment to do that. Ciao
          Hide
          Red Morris added a comment -

          I'd be a little concerned that the file system starts to look like Blackboards and gets confusing to navigate in the FS explorer.

          How about a halfway that caps the number of sub folders to 1000 but remains easy to navigate?

          If you took the ID 1123 and replaced the last 3 numbers with zeros, or rounded it down, and created another string with 999 added to it you get '1000' and '1999', concatenate these and add the ID after a slash you'd have '/1000-1999/1123/'. This would be infinitely scalable, and I think a lot easier to nativage 'by eye'.

          Show
          Red Morris added a comment - I'd be a little concerned that the file system starts to look like Blackboards and gets confusing to navigate in the FS explorer. How about a halfway that caps the number of sub folders to 1000 but remains easy to navigate? If you took the ID 1123 and replaced the last 3 numbers with zeros, or rounded it down, and created another string with 999 added to it you get '1000' and '1999', concatenate these and add the ID after a slash you'd have '/1000-1999/1123/'. This would be infinitely scalable, and I think a lot easier to nativage 'by eye'.
          Hide
          Iñaki Arenaza added a comment -

          The attached patch performs what Red Morris suggests: 1000 users per directory at most, with an easier to navigate directory tree. It uses a 3 level deep structure (which allows for up to 1,000,000,000 users

          So a user with an id of 123,456,789 would have a private directory like:

          $CFG->dataroot/users/123000000-123999999/123456000-123456999/123456789

          and a user with an id of 1,234 a directory like:

          $CFG->dataroot/users/0-999999/1000-1999/1234

          As the code only uses integer multiplications and divisions, it should be faster than dealing with non-decimal bases, spliting and rejoining strings, etc.

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - The attached patch performs what Red Morris suggests: 1000 users per directory at most, with an easier to navigate directory tree. It uses a 3 level deep structure (which allows for up to 1,000,000,000 users So a user with an id of 123,456,789 would have a private directory like: $CFG->dataroot/users/123000000-123999999/123456000-123456999/123456789 and a user with an id of 1,234 a directory like: $CFG->dataroot/users/0-999999/1000-1999/1234 As the code only uses integer multiplications and divisions, it should be faster than dealing with non-decimal bases, spliting and rejoining strings, etc. Saludos. Iñaki.
          Hide
          Martin Dougiamas added a comment -

          A little complex .... how about just:

          /user/xxxx/yyyy

          1000 containing folders 1-1000
          2000 containing folders 1001 - 2000
          3000 containing folders 2001 - 3000
          ....

          Show
          Martin Dougiamas added a comment - A little complex .... how about just: /user/xxxx/yyyy 1000 containing folders 1-1000 2000 containing folders 1001 - 2000 3000 containing folders 2001 - 3000 ....
          Hide
          Martin Dougiamas added a comment -

          OK, so this is suddenly of major importance because I just discovered the ext3 limit is 32000 directories and moodle.org hit that limit some months ago!

          Nicolas, can you do this please?

          On reflection I think it's even simpler if the first-level names are the LOWER bound of the 1000 range (first one will be 0)

          So, here are the steps:

          1) make_user_directory($userid=45678) which will create moodledata/user/45000/45678 and RETURN the full path '45000/45678'
          Note use of the 'user' directory which is different from the current "users" directory.
          2) Write an upgrade.php routine to:
          a) go through the list of directories under the old 'users'
          b) for each name, call the new function make_user_directory
          c) move the contents of the old directory to the just-created one. You need to use PHP functions for this (not shell) for reliability.
          d) put a little README.txt in the old users directory to explain that this is OLD and no longer used. Don't delete it.
          3) Identify all the places that save user images (I don't think there's many) and call make_user_directory to find/create the proper directory there before saving.
          4) Rewrite user/pix.php to look in the new place. No need to change how it's called though. eg http://moodle.org/user/pix.php/1111/f1.jpg is fine.
          5) Test test test
          6) Put into 1.8, 1.9 and HEAD

          Show
          Martin Dougiamas added a comment - OK, so this is suddenly of major importance because I just discovered the ext3 limit is 32000 directories and moodle.org hit that limit some months ago! Nicolas, can you do this please? On reflection I think it's even simpler if the first-level names are the LOWER bound of the 1000 range (first one will be 0) So, here are the steps: 1) make_user_directory($userid=45678) which will create moodledata/user/45000/45678 and RETURN the full path '45000/45678' Note use of the 'user' directory which is different from the current "users" directory. 2) Write an upgrade.php routine to: a) go through the list of directories under the old 'users' b) for each name, call the new function make_user_directory c) move the contents of the old directory to the just-created one. You need to use PHP functions for this (not shell) for reliability. d) put a little README.txt in the old users directory to explain that this is OLD and no longer used. Don't delete it. 3) Identify all the places that save user images (I don't think there's many) and call make_user_directory to find/create the proper directory there before saving. 4) Rewrite user/pix.php to look in the new place. No need to change how it's called though. eg http://moodle.org/user/pix.php/1111/f1.jpg is fine. 5) Test test test 6) Put into 1.8, 1.9 and HEAD
          Hide
          Martin Dougiamas added a comment -

          Yu and Petr, can you just check this plan and make sure I haven't forgotten anything?

          Show
          Martin Dougiamas added a comment - Yu and Petr, can you just check this plan and make sure I haven't forgotten anything?
          Hide
          Tim Hunt added a comment -

          Look OK to me.

          Are't you worried about what happens when moodle.org has 32 million users though

          Show
          Tim Hunt added a comment - Look OK to me. Are't you worried about what happens when moodle.org has 32 million users though
          Hide
          Martin Dougiamas added a comment -

          I am, Tim, but it's further down on the list now.

          Show
          Martin Dougiamas added a comment - I am, Tim, but it's further down on the list now.
          Hide
          Nicolas Connault added a comment -

          All implemented and tested

          Show
          Nicolas Connault added a comment - All implemented and tested
          Hide
          Ryan Smith added a comment -

          Is this fully working? I just updated my 1.83+ site from MOODLE_18_STABLE and the avatars are shown correctly in the Online users block, but when I click on a user, the avatar shown is the standard smiley face moodle avatar.

          Show
          Ryan Smith added a comment - Is this fully working? I just updated my 1.83+ site from MOODLE_18_STABLE and the avatars are shown correctly in the Online users block, but when I click on a user, the avatar shown is the standard smiley face moodle avatar.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I guess (because I haven't seen any change in code), that both backup & restore aren't handling this properly. More exactly these functions:

          • backup_copy_user_files() in backup
          • restore_user_files() in restore (note that this function must accept old backups)

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I guess (because I haven't seen any change in code), that both backup & restore aren't handling this properly. More exactly these functions: backup_copy_user_files() in backup restore_user_files() in restore (note that this function must accept old backups) Ciao
          Hide
          Nicolas Connault added a comment -

          I suppose we could implement slices of 10,000 instead of 1,000. It would allow for 320 million users, a figure which will most likely never be reached, and still fit within the 32,000 directories hard limit within each slice.

          Show
          Nicolas Connault added a comment - I suppose we could implement slices of 10,000 instead of 1,000. It would allow for 320 million users, a figure which will most likely never be reached, and still fit within the 32,000 directories hard limit within each slice.
          Hide
          Martin Dougiamas added a comment -

          Backup ... restore .... ?

          Show
          Martin Dougiamas added a comment - Backup ... restore .... ?
          Hide
          Ryan Smith added a comment -

          I tested the latest stable (1.83+) for today, and I'm still having the same problem.

          With the pix.php file, version 1.16.2.1, that was modified for this bug, it doesn't load any user avatars, all avatars are displayed as the default Moodle smiley face icon. I rolled back to pix.php version 1.16 and the avatars are properly loaded from the moodledata folder.

          Show
          Ryan Smith added a comment - I tested the latest stable (1.83+) for today, and I'm still having the same problem. With the pix.php file, version 1.16.2.1, that was modified for this bug, it doesn't load any user avatars, all avatars are displayed as the default Moodle smiley face icon. I rolled back to pix.php version 1.16 and the avatars are properly loaded from the moodledata folder.
          Hide
          Nicolas Connault added a comment -

          Backup restore complete in head, coming up in 1.9 and 1.8 (just waiting for CVS update before I commit)

          Show
          Nicolas Connault added a comment - Backup restore complete in head, coming up in 1.9 and 1.8 (just waiting for CVS update before I commit)
          Hide
          Oswald Zangerle added a comment - - edited

          After updating backup restore files in 1.8.3+ there is no change at all.

          Show
          Oswald Zangerle added a comment - - edited After updating backup restore files in 1.8.3+ there is no change at all.
          Hide
          Martin Dougiamas added a comment -

          Nicolas, can you please attend to 1.8 urgently ... it's supposed to be stable but people are reporting problems now ...

          Show
          Martin Dougiamas added a comment - Nicolas, can you please attend to 1.8 urgently ... it's supposed to be stable but people are reporting problems now ...
          Hide
          Andrea Bicciolo added a comment -

          Currently the upgrade to 1.8.3+ fails to copy the folders from "moodledata/users" to "moodledata/user/0". We upgraded several sites both on Fedora 6 and Debian Sarge and none copied the user's image folders on the new place.

          In all upgrades it has been needed to do it manually.

          Show
          Andrea Bicciolo added a comment - Currently the upgrade to 1.8.3+ fails to copy the folders from "moodledata/users" to "moodledata/user/0". We upgraded several sites both on Fedora 6 and Debian Sarge and none copied the user's image folders on the new place. In all upgrades it has been needed to do it manually.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yep,

          MDL-10905 is really dangerous for any site generating the "0" folder. And it's implemented in a lot of branches. Need fix ASAP IMO.

          Show
          Eloy Lafuente (stronk7) added a comment - Yep, MDL-10905 is really dangerous for any site generating the "0" folder. And it's implemented in a lot of branches. Need fix ASAP IMO.
          Hide
          Petr Škoda added a comment -

          fix is in MOODLE_18_STABLE cvs and please do not commit trailing whitespace

          Anyway what happens in 1.9/2.0, is the upgrade executed again? What if somebody deletes the picture, would it be copied again from users to user?

          Show
          Petr Škoda added a comment - fix is in MOODLE_18_STABLE cvs and please do not commit trailing whitespace Anyway what happens in 1.9/2.0, is the upgrade executed again? What if somebody deletes the picture, would it be copied again from users to user?
          Hide
          Petr Škoda added a comment -

          And please fix the version numbers in MOODLE_19_STABLE the /version.php and lib/db/upgrade.php does not match! Also please keep HEAD and MOODLE_19_STABLE versions in sync for a while, this is important in case somebody accidentally upgrades to 2.0 and it also prevents problems with upgrade in 2.0 (we must not execute the upgrade code both in 1.9 and 2.0).

          Show
          Petr Škoda added a comment - And please fix the version numbers in MOODLE_19_STABLE the /version.php and lib/db/upgrade.php does not match! Also please keep HEAD and MOODLE_19_STABLE versions in sync for a while, this is important in case somebody accidentally upgrades to 2.0 and it also prevents problems with upgrade in 2.0 (we must not execute the upgrade code both in 1.9 and 2.0).
          Hide
          Oswald Zangerle added a comment -

          In 1.8.3+ (2007021532) it works fine now. Thanks!

          Show
          Oswald Zangerle added a comment - In 1.8.3+ (2007021532) it works fine now. Thanks!
          Hide
          Andrea Bicciolo added a comment -

          The fix seems not working for sites already migrated to 1.8.3+ prior to the fix availability: sites where profile images has been compromised by the new user/0 folder still does not shows the images.

          Show
          Andrea Bicciolo added a comment - The fix seems not working for sites already migrated to 1.8.3+ prior to the fix availability: sites where profile images has been compromised by the new user/0 folder still does not shows the images.
          Hide
          Nicolas Connault added a comment -

          Sorry about the issues. The version mismatch is now solved in 1.9, and 2.0 is in sync with it. However, applying this particular upgrade twice will not cause any problems: the files in the old users directory that are now missing will not cause the new ones to be changed or deleted. The upgrade will simply have no effect.

          Show
          Nicolas Connault added a comment - Sorry about the issues. The version mismatch is now solved in 1.9, and 2.0 is in sync with it. However, applying this particular upgrade twice will not cause any problems: the files in the old users directory that are now missing will not cause the new ones to be changed or deleted. The upgrade will simply have no effect.
          Hide
          Nicolas Connault added a comment -

          Should be fine now

          Show
          Nicolas Connault added a comment - Should be fine now
          Hide
          Martin Dougiamas added a comment -

          Nicolas, just to shorten the upgrade time from 1.8.3+ to 1.9 ... can you add some code to the upgrade in 1.9 and HEAD so that if the new 'user' folder already exists then the "users" upgrade is simply skipped?

          Show
          Martin Dougiamas added a comment - Nicolas, just to shorten the upgrade time from 1.8.3+ to 1.9 ... can you add some code to the upgrade in 1.9 and HEAD so that if the new 'user' folder already exists then the "users" upgrade is simply skipped?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          From Andrea's post above, it seems that sites that have executed the previous upgrade have the "0" folder not created (nor those target user dirs copied).

          Would be a good idea to add one more upgrade block in order to check if the "0" dir has been created and copy missing target users there?

          I know it's "only" 1/1000 of users, hehe, but right now their images are in the "limbo", if I'm not wrong.

          Just one idea, if effectively, those "0" dirs weren't created and filled for early upgraders.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - From Andrea's post above, it seems that sites that have executed the previous upgrade have the "0" folder not created (nor those target user dirs copied). Would be a good idea to add one more upgrade block in order to check if the "0" dir has been created and copy missing target users there? I know it's "only" 1/1000 of users, hehe, but right now their images are in the "limbo", if I'm not wrong. Just one idea, if effectively, those "0" dirs weren't created and filled for early upgraders. Ciao
          Hide
          Anthony Borrow added a comment -

          Nicholas - Are there any plans to merge these changes into 17STABLE? I was reading http://moodle.org/mod/forum/discuss.php?d=82610 and I wondered if it would be worth merging into 1.7. Peace - Anthony

          Show
          Anthony Borrow added a comment - Nicholas - Are there any plans to merge these changes into 17STABLE? I was reading http://moodle.org/mod/forum/discuss.php?d=82610 and I wondered if it would be worth merging into 1.7. Peace - Anthony
          Hide
          Petr Škoda added a comment -

          Please no more non-critical patching in 1.7.x and earlier, we should work on 1.9dev fixing instead

          Show
          Petr Škoda added a comment - Please no more non-critical patching in 1.7.x and earlier, we should work on 1.9dev fixing instead
          Hide
          Petr Škoda added a comment -

          reopening - the restore related code does not work

          Show
          Petr Škoda added a comment - reopening - the restore related code does not work
          Hide
          Petr Škoda added a comment -

          should be fixed in cvs - tested both new and old backups

          Show
          Petr Škoda added a comment - should be fixed in cvs - tested both new and old backups
          Hide
          Nicolas Connault added a comment -

          A new script, admin/fixuserpix.php, has been added to 1.8 and 1.9. It can either be run in the browser (if logged as admin user) or from CLI (with admin user credentials).

          Show
          Nicolas Connault added a comment - A new script, admin/fixuserpix.php, has been added to 1.8 and 1.9. It can either be run in the browser (if logged as admin user) or from CLI (with admin user credentials).

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: