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: AJAX and 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

          Issue Links

            Activity

            ewout Ewout ter Haar created issue -
            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?
            salvetore Michael de Raadt made changes -
            Field Original Value New Value
            Fix Version/s STABLE backlog [ 10463 ]
            Labels triaged
            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
            skodak Petr Skoda made changes -
            Assignee Rajesh Taneja [ rajeshtaneja ] Petr Škoda (skodak) [ skodak ]
            skodak Petr Skoda made changes -
            Fix Version/s DEV backlog [ 10464 ]
            Fix Version/s STABLE backlog [ 10463 ]
            skodak Petr Skoda made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            skodak Petr Skoda made changes -
            Link This issue is blocked by MDL-32050 [ MDL-32050 ]
            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.
            skodak Petr Skoda made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Pull Master Diff URL https://github.com/skodak/moodle/compare/w12_MDL-32050_m23_minify...w12_MDL-31685_m23_filelocking
            Pull Master Branch w12_MDL-31685_m23_filelocking
            Pull from Repository git://github.com/skodak/moodle.git
            Fix Version/s 2.3 [ 10657 ]
            Fix Version/s DEV backlog [ 10464 ]
            Testing Instructions 1/ no idea how to test this, code review should be hopefully enough
            skodak Petr Skoda made changes -
            Link This issue has a non-specific relationship to MDL-31904 [ MDL-31904 ]
            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
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            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.
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            poltawski Dan Poltawski made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester poltawski
            Hide
            poltawski Dan Poltawski added a comment -

            Looks good.

            Show
            poltawski Dan Poltawski added a comment - Looks good.
            poltawski Dan Poltawski made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            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
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 29/Mar/12
            Subversion JIRA

            Links Hierarchy

             Documentation

            Invalid license: EXPIRED

              People

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

                Dates

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