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

Redis: exception being silenced after connection retries

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Integration review in progress
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 3.5.5
    • Fix Version/s: None
    • Component/s: Authentication
    • Database:
      Any
    • Testing Instructions:
      Hide

      PHPUNIT:

      1) Install redis:

      https://docs.moodle.org/36/en/Redis_cache_store

       
      2) Add this to config.php

      define('TEST_SESSION_REDIS_HOST', '127.0.0.1');
      $CFG->session_handler_class = '\core\session\redis';
      $CFG->session_redis_host = '127.0.0.1';
      $CFG->session_redis_acquire_lock_timeout = 120;
      $CFG->session_redis_lock_expire = 7200;

       

      3) Run unit tests:

      vendor/bin/phpunit lib/tests/session_redis_test.php

       

      NOTE: The redis unit tests prior to this patch did not pass, and I am assuming were skipped in the HQ CI because the redis & drivers were not installed. Can we also fix CIME so that these are included?

       

      Manual testing:

      1) Install redis https://docs.moodle.org/36/en/Redis_cache_store

      2) Add this to config.php

       

      define('TEST_SESSION_REDIS_HOST', '127.0.0.1');
      $CFG->session_handler_class = '\core\session\redis';
      $CFG->session_redis_host = '127.0.0.1';
      $CFG->session_redis_port = 111111;  // Optional.
      $CFG->session_redis_database = 0;  // Optional, default is db 0.
      $CFG->session_redis_prefix = ''; // Optional, default is don't set one.
      $CFG->session_redis_acquire_lock_timeout = 120;
      $CFG->session_redis_lock_expire = 7200;
      

      3) Load any Moodle page in browser. 

       there should be an exception 

       

      Exception - Failed to connect (try 5 out of 5) to redis at 127.0.0.1:111111, error returned was: Unable to connect to host.

       

      It can be slightly different depending on redis version. But the important part is that it exceeds number of attemts - i.e. did 5 out of 5 and failed

       

      Show
      PHPUNIT: 1) Install redis: https://docs.moodle.org/36/en/Redis_cache_store   2) Add this to config.php define( 'TEST_SESSION_REDIS_HOST' , '127.0.0.1' ); $CFG->session_handler_class = '\core\session\redis' ; $CFG->session_redis_host = '127.0.0.1' ; $CFG->session_redis_acquire_lock_timeout = 120 ; $CFG->session_redis_lock_expire = 7200 ;   3) Run unit tests: vendor/bin/phpunit lib/tests/session_redis_test.php   NOTE: The redis unit tests prior to this patch did not pass, and I am assuming were skipped in the HQ CI because the redis & drivers were not installed. Can we also fix CIME so that these are included?   Manual testing: 1) Install redis  https://docs.moodle.org/36/en/Redis_cache_store 2) Add this to config.php   define( 'TEST_SESSION_REDIS_HOST' , '127.0.0.1' ); $CFG->session_handler_class = '\core\session\redis' ; $CFG->session_redis_host = '127.0.0.1' ; $CFG->session_redis_port = 111111 ; // Optional. $CFG->session_redis_database = 0 ; // Optional, default is db 0. $CFG->session_redis_prefix = '' ; // Optional, default is don't set one. $CFG->session_redis_acquire_lock_timeout = 120 ; $CFG->session_redis_lock_expire = 7200 ; 3) Load any Moodle page in browser.   there should be an exception    Exception - Failed to connect (try 5 out of 5) to redis at 127.0.0.1:111111, error returned was: Unable to connect to host.   It can be slightly different depending on redis version. But the important part is that it exceeds number of attemts - i.e. did 5 out of 5 and failed  
    • Affected Branches:
      MOODLE_35_STABLE
    • Pull 3.5 Branch:
      MDL-65249-redis-catch-exception-after-retry-MOODLE_35_STABLE
    • Pull 3.6 Branch:
      MDL-65249-redis-catch-exception-after-retry-MOODLE_36_STABLE
    • Pull Master Branch:
      MDL-65249-redis-catch-exception-after-retry-master

      Description

      It looks like a bug has been introduced in patch MDL-59866

      So if it exceeds number of retries to establish a connection it simply returns false with no exception. It used to throw an exception before the patch, where as now it continues and tries to create a session when it simply can't, and results in undefined behavior.

       

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 1 hour, 30 minutes
                  1h 30m