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

don't use filesize when reading from cache_filestore

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.5, 2.5
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Caching
    • Labels:
      None

      Description

      cache_filestore has a hack, suggesting that PHPUnit tests fail, but normal operation is correct.

      My estimation of the situation is that under PHPUnit, clearstatcache() wasn't called at some point, which required the guess of the larger size. The current PHPUnit tests pass without the hack.

      However all this exposes the issue of race conditions and caching between what the filesize is and what's actually read. To remove that, we will not just use fread to collect all the data until the end of file is reached.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              matteo Matteo Scaramuccia added a comment -

              Hi Russel,
              in MDL-41058 you'll find a case (NFS-based dataroot) where this fix could fit too, maybe there it should be required some other improvements to avoid the usage of flock().

              Show
              matteo Matteo Scaramuccia added a comment - Hi Russel, in MDL-41058 you'll find a case (NFS-based dataroot) where this fix could fit too, maybe there it should be required some other improvements to avoid the usage of flock() .
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Looks superb thanks Russell and all unit tests passing.

              I've put this straight up for integration review, if you get a chance could you please produce stable branches.
              Integrator this cherry-picks cleanly.

              Many thanks
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Looks superb thanks Russell and all unit tests passing. I've put this straight up for integration review, if you get a chance could you please produce stable branches. Integrator this cherry-picks cleanly. Many thanks Sam
              Hide
              mr-russ Russell Smith added a comment -

              Added back-branches. These were clean cherry-picks of master.

              Show
              mr-russ Russell Smith added a comment - Added back-branches. These were clean cherry-picks of master.
              Hide
              poltawski Dan Poltawski added a comment -

              Integrated to master, 25 and 24 - thanks Russell!

              Show
              poltawski Dan Poltawski added a comment - Integrated to master, 25 and 24 - thanks Russell!
              Hide
              phalacee Jason Fowler added a comment -

              Worked fine Russell. Thanks for the patch

              Show
              phalacee Jason Fowler added a comment - Worked fine Russell. Thanks for the patch
              Hide
              damyon Damyon Wiese added a comment -

              There was a young man named McGee
              Who thought squashing bugs was easy
              He tried it one day
              And to his dismay
              The bug guts made his keyboard all greasy

              Thanks!

              This has issue has been fixed and released in Moodle.

              Show
              damyon Damyon Wiese added a comment - There was a young man named McGee Who thought squashing bugs was easy He tried it one day And to his dismay The bug guts made his keyboard all greasy Thanks! This has issue has been fixed and released in Moodle.

                People

                • Votes:
                  1 Vote for this issue
                  Watchers:
                  5 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    9/Sep/13