Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31685

Creation of empty javascript cachefiles: Minify library, Lustre and NFS filesystems

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: JavaScript
    • Labels:

      Description

      I had a problem with empty cache files being created [1] on a Lustre filesystem [2].

      I solved my particular problem by changing Minify::setCache($cachedir, true); to Minify::setCache($cachedir, false); in /lib/javascript.php . This instructs the Minify library to not use file-locking (which is also the default of this library).

      There are two issues: 1. I think in the configuration of /lib/minify/config.php should be used in /lib/javascript (or, clarifying that settings in config.php are not used). 2. Why are we using file_locking as the hard-coded default? I don't understand the trade-offs, but note the upstream library doesn't use file locking as the default. Also note that acording to the post in [1], the same problem seems to happen with NFS (I did not test this).

      [1] http://moodle.org/mod/forum/discuss.php?d=163122#p717490
      [2] www.lustre.org

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              ewout Ewout ter Haar added a comment -

              So my reasoning is: if there are systems out there that do not tolerate locking why not change the default to "no locking" in Moodle, just like the Minify library does?

              But I must say, I still don't really understand why the zero-length files were created in the first place: I looked a bit at the Minify cache code and aparently it checks wether the cache-files were created correctly. But that check doesn't seem to be effective.

              Show
              ewout Ewout ter Haar added a comment - So my reasoning is: if there are systems out there that do not tolerate locking why not change the default to "no locking" in Moodle, just like the Minify library does? But I must say, I still don't really understand why the zero-length files were created in the first place: I looked a bit at the Minify cache code and aparently it checks wether the cache-files were created correctly. But that check doesn't seem to be effective.
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting that and suggesting a solution.

              Could you describe the problem that was caused by this? What went wrong?

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting that and suggesting a solution. Could you describe the problem that was caused by this? What went wrong?
              Hide
              ewout Ewout ter Haar added a comment -

              Yes, good point, I didn't describe the original problem that led to the empty cache files diagnostic. It's simple, really, an empty javascript-static.js means the YUI is not loaded and nothing involving javascript works (the whole navegation block becomes unresponsive, for example).

              There is a catch though: client-side caching may mask the problem for long time!

              Show
              ewout Ewout ter Haar added a comment - Yes, good point, I didn't describe the original problem that led to the empty cache files diagnostic. It's simple, really, an empty javascript-static.js means the YUI is not loaded and nothing involving javascript works (the whole navegation block becomes unresponsive, for example). There is a catch though: client-side caching may mask the problem for long time!
              Hide
              skodak Petr Skoda added a comment -

              Hi, locking may be necessary on servers with very high load, in fact we are going to add more file based locking soon. I am going to add new config-dist.php option that disables all file locking (not recommended for production sites).

              PEtr

              Show
              skodak Petr Skoda added a comment - Hi, locking may be necessary on servers with very high load, in fact we are going to add more file based locking soon. I am going to add new config-dist.php option that disables all file locking (not recommended for production sites). PEtr
              Hide
              skodak Petr Skoda added a comment -

              It is also possible to use new temp dir and cache dir on local fast filesystem that usually supports locking.

              See MDL-31904 that describes lang related caching problem that will be resolved soon by file locking (using a bit different patch), it will use the same disabler, but the workaround will be again the same.

              Thanks for the report.

              Show
              skodak Petr Skoda added a comment - It is also possible to use new temp dir and cache dir on local fast filesystem that usually supports locking. See MDL-31904 that describes lang related caching problem that will be resolved soon by file locking (using a bit different patch), it will use the same disabler, but the workaround will be again the same. Thanks for the report.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              samhemelryk Sam Hemelryk added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              skodak Petr Skoda added a comment -

              rebased for integration

              Show
              skodak Petr Skoda added a comment - rebased for integration
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Petr this has been integrated now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Petr this has been integrated now.
              Hide
              poltawski Dan Poltawski added a comment -

              Looks good.

              Show
              poltawski Dan Poltawski added a comment - Looks good.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              And this has landed upstream, finally! Yay!

              תודה רבה && شكرا جزيلا



              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12