Moodle
  1. Moodle
  2. MDL-36322

All caching plugins need to cope with caching servers being down

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Caching
    • Labels:
    • Testing Instructions:
      Hide

      Test 1

      1. Set debugging level to DEVELOPER, eventually w/ debugdisplay
      2. First, try the steps described in the description
      3. Add a memcache* store using a correct configuration
      4. Stop the memcached service serving that store
      5. Browse Site administration ► Plugins ► Caching ► Configuration again: no error about incorrect flushing the memcache* instance should be reported
      6. Try to update Moodle: no error about incorrect flushing against "stopped"/malconfigured stores should be reported too

      Test 2.

      1. Set up the test instance details for memcache, memcached, and mongodb.
      2. Browse to Settings > Plugins > Caching > Test performance.
      3. Make sure you get no errors.
      Show
      Test 1 Set debugging level to DEVELOPER , eventually w/ debugdisplay First, try the steps described in the description Add a memcache* store using a correct configuration Stop the memcached service serving that store Browse Site administration ► Plugins ► Caching ► Configuration again: no error about incorrect flushing the memcache* instance should be reported Try to update Moodle: no error about incorrect flushing against "stopped"/malconfigured stores should be reported too Test 2. Set up the test instance details for memcache, memcached, and mongodb. Browse to Settings > Plugins > Caching > Test performance. Make sure you get no errors.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-MDL-36322-m24
    • Pull Master Branch:
      wip-MDL-36322-m25
    • Rank:
      45115

      Description

      1. Add a mongodb cache instance (give it bad config)
      2. Change databasemeta to use it

      Expected result:
      Doesn't kill site

      Actual result:
      Exception - couldn't connect to any servers in the list
      Debug info:
      Error code: generalexceptionmessage
      Stack trace:
      line 256 of /cache/stores/mongodb/lib.php: MongoCursorException thrown
      line 256 of /cache/stores/mongodb/lib.php: call to MongoCollection->findOne()
      line 292 of /cache/classes/loaders.php: call to cachestore_mongodb->get()
      line 1272 of /cache/classes/loaders.php: call to cache->get()
      line 401 of /lib/dml/pgsql_native_moodle_database.php: call to cache_application->get()
      line 533 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_columns()
      line 1330 of /lib/dml/moodle_database.php: call to moodle_database->where_clause()
      line 5879 of /lib/accesslib.php: call to moodle_database->get_record()
      line 7139 of /lib/accesslib.php: call to context_system::instance()
      line 699 of /lib/setup.php: call to get_system_context()
      line 100 of /config.php: call to require_once()
      line 28 of /cache/admin.php: call to require_once()

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          Just generalising this a bit

          Show
          Martin Dougiamas added a comment - Just generalising this a bit
          Hide
          Dan Poltawski added a comment -

          Notice: Memcache::flush(): Server 127.0.0.1 (tcp 11211) failed with: Connection refused (61) in /Users/danp/git/integration/cache/stores/memcache/lib.php on line 306

          Show
          Dan Poltawski added a comment - Notice: Memcache::flush(): Server 127.0.0.1 (tcp 11211) failed with: Connection refused (61) in /Users/danp/git/integration/cache/stores/memcache/lib.php on line 306
          Hide
          Sam Hemelryk added a comment -

          Thanks for the report Dan,

          This is most certainly a bug.
          The cache system has a couple of methods to make sure that the plugin type is usable and that any given configured instance is in a state ready to be used.
          I imagine there are two things to be looked at by this issue:

          1. That we are using those methods before calling plugin static methods, and before using an initialised instance.
          2. That the mongodb store, as well as the other stores of similar nature, and testing the connection as part of their initialisation and replying to inquiries about their state accordingly.

          I've marked this triaged and raised its priority to major.
          Feel free to let me know if you think I've got that wrong.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks for the report Dan, This is most certainly a bug. The cache system has a couple of methods to make sure that the plugin type is usable and that any given configured instance is in a state ready to be used. I imagine there are two things to be looked at by this issue: That we are using those methods before calling plugin static methods, and before using an initialised instance. That the mongodb store, as well as the other stores of similar nature, and testing the connection as part of their initialisation and replying to inquiries about their state accordingly. I've marked this triaged and raised its priority to major. Feel free to let me know if you think I've got that wrong. Many thanks Sam
          Hide
          Matteo Scaramuccia added a comment -

          Hi Sam,
          this morning, while updating my VM, I got the same error:

          [Sat Nov 17 10:28:46 2012] [error] [client 192.168.0.100] PHP Notice:  MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/login/index.php
          [Sat Nov 17 10:28:56 2012] [error] [client 192.168.0.100] PHP Notice:  MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php
          [Sat Nov 17 10:29:04 2012] [error] [client 192.168.0.100] PHP Notice:  MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php
          [Sat Nov 17 10:29:20 2012] [error] [client 192.168.0.100] PHP Notice:  MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php?confirmupgrade=1&confirmrelease=1
          

          The issue is that purge() for memcache store doesn't perform a check about the instance being ready to connect and that check should be performed using Memcache::connect since Memcache::addServer doesn't connect until required. I'll try to give you a patch proposal, maybe looking also at mongodb code too.

          Show
          Matteo Scaramuccia added a comment - Hi Sam, this morning, while updating my VM, I got the same error: [Sat Nov 17 10:28:46 2012] [error] [client 192.168.0.100] PHP Notice: MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/login/index.php [Sat Nov 17 10:28:56 2012] [error] [client 192.168.0.100] PHP Notice: MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php [Sat Nov 17 10:29:04 2012] [error] [client 192.168.0.100] PHP Notice: MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php [Sat Nov 17 10:29:20 2012] [error] [client 192.168.0.100] PHP Notice: MemcachePool::flush(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /var/www/moodle-master-git-remote/cache/stores/memcache/lib.php on line 306, referer: http://hostname/moodle-master/admin/index.php?confirmupgrade=1&confirmrelease=1 The issue is that purge() for memcache store doesn't perform a check about the instance being ready to connect and that check should be performed using Memcache::connect since Memcache::addServer doesn't connect until required. I'll try to give you a patch proposal, maybe looking also at mongodb code too.
          Hide
          Matteo Scaramuccia added a comment -

          Hi Sam,
          here it is, tested with a memcache cache store being memcached stopped: https://github.com/scara/moodle/compare/master...wip_m24_MDL-36322_Errors_when_purging_memcaches_mongodb_stores_during_updates.

          HTH,
          Matteo

          Show
          Matteo Scaramuccia added a comment - Hi Sam, here it is, tested with a memcache cache store being memcached stopped: https://github.com/scara/moodle/compare/master...wip_m24_MDL-36322_Errors_when_purging_memcaches_mongodb_stores_during_updates . HTH, Matteo
          Hide
          Matteo Scaramuccia added a comment -

          Hi Sam,
          when the new abstract class (MDL-36768) will land into the main stream (moodle.git) I'll update my proposal by assigning me the issue and then ask you for a peer review .

          HTH,
          Matteo

          Show
          Matteo Scaramuccia added a comment - Hi Sam, when the new abstract class ( MDL-36768 ) will land into the main stream ( moodle.git ) I'll update my proposal by assigning me the issue and then ask you for a peer review . HTH, Matteo
          Hide
          Matteo Scaramuccia added a comment -

          Hi Sam,
          here is my proposal rebased to 2.4rc1.

          Show
          Matteo Scaramuccia added a comment - Hi Sam, here is my proposal rebased to 2.4rc1.
          Hide
          Matteo Scaramuccia added a comment -

          Hi Sam
          this is just a ping at the end of the year as well as a test to see if notifications now work (see MDLSITE-2066).

          Show
          Matteo Scaramuccia added a comment - Hi Sam this is just a ping at the end of the year as well as a test to see if notifications now work (see MDLSITE-2066 ).
          Hide
          Sam Hemelryk added a comment -

          Again sorry about this sitting around for so long!

          Show
          Sam Hemelryk added a comment - Again sorry about this sitting around for so long!
          Hide
          Sam Hemelryk added a comment -

          Thanks Matteo,

          Things looked good. The memcache stores did have an error but that was easily fixed.
          I've created a couple of branches based upon your branches and am putting this up for integration review now.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Matteo, Things looked good. The memcache stores did have an error but that was easily fixed. I've created a couple of branches based upon your branches and am putting this up for integration review now. Many thanks Sam
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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
          Matteo Scaramuccia added a comment - - edited

          Hi Sam,
          just pulled from master, going to http://hostname/oath/to/cache/testperformance.php, I'm still receiving (debugging DEVELOPER):

          Notice: MemcachePool::set(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /path/to/moodle-master-git-remote/cache/stores/memcache/lib.php on line 226
          

          I'll look at the code and eventually open a new issue.

          Edit: opppsss... it's not been integrated yet! Sorry All for my silly noise... Shame on me

          Show
          Matteo Scaramuccia added a comment - - edited Hi Sam, just pulled from master , going to http://hostname/oath/to/cache/testperformance.php , I'm still receiving (debugging DEVELOPER ): Notice: MemcachePool::set(): Server localhost (tcp 11211, udp 0) failed with: Connection refused (111) in /path/to/moodle-master-git-remote/cache/stores/memcache/lib.php on line 226 I'll look at the code and eventually open a new issue. Edit: opppsss... it's not been integrated yet! Sorry All for my silly noise... Shame on me
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Matteo, can you try now?

          PS: Joking, It's still not integrated, LOL (sorry, couldn't resist). Thanks for your collaboration!

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Matteo, can you try now? PS: Joking, It's still not integrated, LOL (sorry, couldn't resist). Thanks for your collaboration!
          Hide
          Matteo Scaramuccia added a comment -

          Touché

          Show
          Matteo Scaramuccia added a comment - Touché
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oki, after a bit of chatting about this with Sam, there are only some leftovers to clarify, before allowing this to land:

          1) If we are moving the connection stuff (when present) from initialise() to constructor... then we need to clarify that in the API and make it clear which are the responsibilities for each method. For example, the mongo store right now explicitly is saying "but does not connect to it" and that's 100% wrong if we apply the patch. So please, try to clarify it in as many places as possible.

          2) There are some clone() operations in create_store_from_config(), and it causes me a bit of stress about all those resources (connections...) being clonable always. Sam did some tests with mongo and seemed to work... but... not sure how to behave about this point (unit test somehow, move it to some other place... not sure).

          Waiting for some feedback... otherwise I think I understand it and looks ok.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oki, after a bit of chatting about this with Sam, there are only some leftovers to clarify, before allowing this to land: 1) If we are moving the connection stuff (when present) from initialise() to constructor... then we need to clarify that in the API and make it clear which are the responsibilities for each method. For example, the mongo store right now explicitly is saying "but does not connect to it" and that's 100% wrong if we apply the patch. So please, try to clarify it in as many places as possible. 2) There are some clone() operations in create_store_from_config(), and it causes me a bit of stress about all those resources (connections...) being clonable always. Sam did some tests with mongo and seemed to work... but... not sure how to behave about this point (unit test somehow, move it to some other place... not sure). Waiting for some feedback... otherwise I think I understand it and looks ok. Ciao
          Hide
          Sam Hemelryk added a comment -

          Hi Eloy,

          Thanks for the two points:

          Re 1: I'll go through code and review/update the docs no probs and clarify the difference. I'll also make mention of it somewhere in the docs.

          Re 2: Unfortunately at present we don't have any unit tests for stores that are not default.
          I'm pretty sure I could knock up some sort of unit tests if we want to go down that path. I've been thinking it over for a while.
          We already have the performance test script that requires a store instance.
          With a little core code change we could ensure that when running unit tests we use those test instances if available then we could write an abstract test class that could be extended by stores to perform any tests we require.

          So what to do right now?

          Presently it is a bit of a conundrum.
          It is expected that a stores initialisation will always succeed.

          If a store isn't going to deal with a connection/property being cloned then there are options (the first two that come to mind):
          a) In the constructor open the connection to test that is works and then close it. Within the initialisation open the connection and store it.
          b) Create a means to open a connection from outside for the cache store, and then have that means hold onto any established connections and hand them out if requested again in the future. In the constructor request a connection to check it works but don't hold onto it, and then in the init request it again.

          So there is a work around to this approach which is good, but we'll need to document it well so that people understand. It's going to mean writing a cache store is a little more advanced but I think it is doable.

          Perhaps the other idea, the alternative to this approach would be to write a means to allow store initialisation to notify of success/failure.
          Upon failure we'd simply not use it, and should all fail we'd have to either use the default store, or use the dummy store (nothing cached really).
          I've not fully gone through the code but I think this wouldn't be too difficult to implement.
          The advantage to this would be that we would not need to change store logic so drastically. They'd just need to return true on success and false on failure.

          So options:

          1. Integrate this now. Create a new issue to create unit tests and test for this case.
          2. Create an issue for unit tests, reopen this and make it dependent on the unit tests being integrated first.
          3. Scrap this change and allow initialisation to fail and deal with it in cache API. Create a new issue to create unit tests and test for this case.

          What are your thoughts Eloy, I'm 50/50 between #1 and #3 presently.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Eloy, Thanks for the two points: Re 1: I'll go through code and review/update the docs no probs and clarify the difference. I'll also make mention of it somewhere in the docs. Re 2: Unfortunately at present we don't have any unit tests for stores that are not default. I'm pretty sure I could knock up some sort of unit tests if we want to go down that path. I've been thinking it over for a while. We already have the performance test script that requires a store instance. With a little core code change we could ensure that when running unit tests we use those test instances if available then we could write an abstract test class that could be extended by stores to perform any tests we require. So what to do right now? Presently it is a bit of a conundrum. It is expected that a stores initialisation will always succeed. If a store isn't going to deal with a connection/property being cloned then there are options (the first two that come to mind): a) In the constructor open the connection to test that is works and then close it. Within the initialisation open the connection and store it. b) Create a means to open a connection from outside for the cache store, and then have that means hold onto any established connections and hand them out if requested again in the future. In the constructor request a connection to check it works but don't hold onto it, and then in the init request it again. So there is a work around to this approach which is good, but we'll need to document it well so that people understand. It's going to mean writing a cache store is a little more advanced but I think it is doable. Perhaps the other idea, the alternative to this approach would be to write a means to allow store initialisation to notify of success/failure. Upon failure we'd simply not use it, and should all fail we'd have to either use the default store, or use the dummy store (nothing cached really). I've not fully gone through the code but I think this wouldn't be too difficult to implement. The advantage to this would be that we would not need to change store logic so drastically. They'd just need to return true on success and false on failure. So options: Integrate this now. Create a new issue to create unit tests and test for this case. Create an issue for unit tests, reopen this and make it dependent on the unit tests being integrated first. Scrap this change and allow initialisation to fail and deal with it in cache API. Create a new issue to create unit tests and test for this case. What are your thoughts Eloy, I'm 50/50 between #1 and #3 presently. Many thanks Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm a bit worried about the proposal of starting the connections just to verify they work and close them immediately, to be reopened again by init. Sounds like a slowdown in my mind, doing the same twice for each request.

          Perhaps one alternative solution maybe to provide to the API one store_clone() (note surely the name is not correct), that, by default will perform a simple clone() but, for stores not supporting cloning, will enable the developer to re-initialize anything?

          Not sure if it has sense or no, just asking... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm a bit worried about the proposal of starting the connections just to verify they work and close them immediately, to be reopened again by init. Sounds like a slowdown in my mind, doing the same twice for each request. Perhaps one alternative solution maybe to provide to the API one store_clone() (note surely the name is not correct), that, by default will perform a simple clone() but, for stores not supporting cloning, will enable the developer to re-initialize anything? Not sure if it has sense or no, just asking... ciao
          Hide
          Sam Hemelryk added a comment -

          Using a method to control cloning sounds like a good option to me as well.

          Worth noting is that the magic method __clone is available as well although that is called immediately after the object has been cloned.
          It would allow an object to do anything it wanted immediately after the clone which I imagine would cover all cases.
          The only situation that it would not cover would be if an error/exception was generated when trying to clone the store. I imagine that would be highly unlikely however (and surely unstable design).

          Either we live with __clone or we implement our own method that would be responsible for running the clone routine.
          It could be done either before or after this gets in, I'm happy either way

          Show
          Sam Hemelryk added a comment - Using a method to control cloning sounds like a good option to me as well. Worth noting is that the magic method __clone is available as well although that is called immediately after the object has been cloned. It would allow an object to do anything it wanted immediately after the clone which I imagine would cover all cases. The only situation that it would not cover would be if an error/exception was generated when trying to clone the store. I imagine that would be highly unlikely however (and surely unstable design). Either we live with __clone or we implement our own method that would be responsible for running the clone routine. It could be done either before or after this gets in, I'm happy either way
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Perhaps we can do that there, so the API only changes once, and everybody gets directions properly? That would mean delay this 1-week, but I think that it's better than doing it in 2 steps.

          So my +1 goes to reopen and implement the missing phpdocs & clone, if that's a solution really.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Perhaps we can do that there, so the API only changes once, and everybody gets directions properly? That would mean delay this 1-week, but I think that it's better than doing it in 2 steps. So my +1 goes to reopen and implement the missing phpdocs & clone, if that's a solution really. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Agreed to give this one more run, so reopening, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Agreed to give this one more run, so reopening, thanks!
          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy, I'm putting this back up for integration review now with two additional commits:

          1. Clarifies the phpdocs for the __construct and initialise methods.
          2. Implements a cache_store::create_clone method to handle the production of a clone. This way any stores with an issue when cloning can override that method to address issues if required.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Eloy, I'm putting this back up for integration review now with two additional commits: Clarifies the phpdocs for the __construct and initialise methods. Implements a cache_store::create_clone method to handle the production of a clone. This way any stores with an issue when cloning can override that method to address issues if required. Many thanks Sam
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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
          Dan Poltawski added a comment -

          Reading through this and looking at the code it looks good to me. I'm gonna get Eloy to +1 it.

          Show
          Dan Poltawski added a comment - Reading through this and looking at the code it looks good to me. I'm gonna get Eloy to +1 it.
          Hide
          Dan Poltawski added a comment -

          Integrated to master and 24. Thanks everyone!

          Show
          Dan Poltawski added a comment - Integrated to master and 24. Thanks everyone!
          Hide
          Rossiani Wijaya added a comment -

          // Taking Andrew's testing issue.

          Tested this issue for 2.4 and Master. It works great. The notices and errors are not longer exist.

          However, with my limitation of understanding caching entirely, I'm not 100% percent sure I did the test correctly. Second opinion or tester for this issue would be great.

          Thanks.

          Rosie

          Show
          Rossiani Wijaya added a comment - // Taking Andrew's testing issue. Tested this issue for 2.4 and Master. It works great. The notices and errors are not longer exist. However, with my limitation of understanding caching entirely, I'm not 100% percent sure I did the test correctly. Second opinion or tester for this issue would be great. Thanks. Rosie
          Hide
          Rossiani Wijaya added a comment -

          Hi Sam,

          Just noting an error when trying to delete MongoDB (with bad config) store instance.

          Exception - Failed to connect to: 127.0.9.1:27017: Authentication failed on database 'admin' with username 'user': test fails
          
          More information about this error
          Debug info:
          Error code: generalexceptionmessage
          Stack trace:
          
              line 466 of /cache/stores/mongodb/lib.php: MongoConnectionException thrown
              line 466 of /cache/stores/mongodb/lib.php: call to Mongo->__construct()
              line 315 of /cache/locallib.php: call to cachestore_mongodb->instance_deleted()
              line 126 of /cache/admin.php: call to cache_config_writer->delete_store_instance()
          
          Show
          Rossiani Wijaya added a comment - Hi Sam, Just noting an error when trying to delete MongoDB (with bad config) store instance. Exception - Failed to connect to: 127.0.9.1:27017: Authentication failed on database 'admin' with username 'user': test fails More information about this error Debug info: Error code: generalexceptionmessage Stack trace: line 466 of /cache/stores/mongodb/lib.php: MongoConnectionException thrown line 466 of /cache/stores/mongodb/lib.php: call to Mongo->__construct() line 315 of /cache/locallib.php: call to cachestore_mongodb->instance_deleted() line 126 of /cache/admin.php: call to cache_config_writer->delete_store_instance()
          Hide
          Dan Poltawski added a comment -

          Looks like we've missed a spot.. I think this should be failed.

          Show
          Dan Poltawski added a comment - Looks like we've missed a spot.. I think this should be failed.
          Hide
          Rossiani Wijaya added a comment -

          Failing this issue due to the MongoDB deletion error.

          Sorry.

          Show
          Rossiani Wijaya added a comment - Failing this issue due to the MongoDB deletion error. Sorry.
          Hide
          Sam Hemelryk added a comment -

          A fix has been integrated for this now. Thanks for picking up the issue Rosie.
          I've moved this back to ready for testing now.

          Show
          Sam Hemelryk added a comment - A fix has been integrated for this now. Thanks for picking up the issue Rosie. I've moved this back to ready for testing now.
          Hide
          Damyon Wiese added a comment -

          I have restested this on 24 and master and the test instructions complete correctly.

          Thanks Sam

          Show
          Damyon Wiese added a comment - I have restested this on 24 and master and the test instructions complete correctly. Thanks Sam
          Hide
          Damyon Wiese added a comment -

          Passing

          Show
          Damyon Wiese added a comment - Passing
          Hide
          Damyon Wiese added a comment -

          Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon

            People

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

              Dates

              • Created:
                Updated:
                Resolved: