Moodle
  1. Moodle
  2. MDL-31685

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

    Details

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

      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

        Issue Links

          Activity

          Hide
          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 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
          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
          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 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 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
          Petr Škoda 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
          Petr Škoda 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
          Petr Škoda 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
          Petr Škoda 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
          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
          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
          Petr Škoda added a comment -

          rebased for integration

          Show
          Petr Škoda added a comment - rebased for integration
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr this has been integrated now.

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

          Looks good.

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

          And this has landed upstream, finally! Yay!

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



          Closing, ciao

          Show
          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: