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

cachestore_redis: Connection appears to do an unnecessary ping()

    XMLWordPrintable

Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 4.0.2
    • 4.1
    • Caching
    • MOODLE_400_STABLE
    • MOODLE_401_STABLE
    • MDL-75369-master
    • Hide

      Testing needs to use a system which is configured to use Redis for (a) user session, and (b) at least one MUC cache, as per these instructions:

      https://docs.moodle.org/400/en/Redis_cache_store
      https://docs.moodle.org/400/en/Session_handling#Redis_session_driver

      There should be no difference in behaviour, so please just:

      • Confirm you can successfully log into Moodle
      • Under Site administration > Development > Debugging, turn on the Performance info option (perfdebug).
      • Click around a few pages and confirm you are still logged in.
      • On a page which exercises at least one Redis MUC cache and where data should be coming from the cache, look at the performance info at the bottom of the screen and confirm there is a cache hit (H) for the Redis cache; i.e. the cache appears working.

      Optionally we can do extra testing to confirm that the ping commands are not happening. There are 2 different ways to do this:

      1) If you have CLI access to Redis:

      a. Run 'info commandstats' and look for the number of calls to ping (note - if the ping line is absent it means there weren't any, you can manually type 'ping' first if you want it to show up):

      > info commandstats
      # Commandstats
      ...
      cmdstat_ping:calls=1,usec=1,usec_per_call=1.00
      ...
      

      b. Make some Moodle web page requests

      c. Run 'info commandstats' again and confirm the number of ping commands is still the same

      2) If you have profiling enabled on your Moodle server

      a. View a course page with profiling on (e.g. add &PROFILEME=1 to the end of the URL, if you have selective profiling enabled).

      b. Look at the profiling result by going to Site administration / Development / Profiling runs, click on the date of the profiling run, then click View profiling details

      c. In the search box at top right, type 'redis'. Verify that you see several commands starting 'Redis::' including 'Redis::connect', but no 'Redis::ping'.

      Show
      Testing needs to use a system which is configured to use Redis for (a) user session, and (b) at least one MUC cache, as per these instructions: https://docs.moodle.org/400/en/Redis_cache_store https://docs.moodle.org/400/en/Session_handling#Redis_session_driver There should be no difference in behaviour, so please just: Confirm you can successfully log into Moodle Under Site administration > Development > Debugging , turn on the Performance info option (perfdebug). Click around a few pages and confirm you are still logged in. On a page which exercises at least one Redis MUC cache and where data should be coming from the cache, look at the performance info at the bottom of the screen and confirm there is a cache hit (H) for the Redis cache; i.e. the cache appears working. Optionally we can do extra testing to confirm that the ping commands are not happening. There are 2 different ways to do this: 1) If you have CLI access to Redis: a. Run 'info commandstats' and look for the number of calls to ping (note - if the ping line is absent it means there weren't any, you can manually type 'ping' first if you want it to show up): > info commandstats # Commandstats ... cmdstat_ping:calls=1,usec=1,usec_per_call=1.00 ... b. Make some Moodle web page requests c. Run 'info commandstats' again and confirm the number of ping commands is still the same 2) If you have profiling enabled on your Moodle server a. View a course page with profiling on (e.g. add &PROFILEME=1 to the end of the URL, if you have selective profiling enabled). b. Look at the profiling result by going to Site administration / Development / Profiling runs , click on the date of the profiling run, then click View profiling details c. In the search box at top right, type 'redis'. Verify that you see several commands starting 'Redis::' including 'Redis::connect', but no 'Redis::ping'.

    Description

      When Moodle connects to Redis (function new_redis in redis/lib.php), it does a ping as well as a connect:

                  if ($redis->connect($server, $port)) {
      ...omitted code that sets some options on the redis object...
                      // Database setting option...
                      $this->isready = $this->ping($redis);
                  } else {
                      $this->isready = false;
                  }
      

      I was profiling something unrelated and noticed that this ping takes almost as long as the connect. in my profile, Redis::connect took a total of 4.1ms for 2 calls (presumably this is because we're using Redis for both session and MUC caches, I didn't check), while Redis::ping took a total of 1.7ms for the 2 calls.

      Obviously the exact time taken will depend on infrastructure but I don't understand the benefit of the ping here. From the time it takes, it seems like the connect() call does really connect, i.e. if it was basically just storing the address and not doing any networking it would complete in well under a millisecond, so if the server is down then that should probably fail already. Also, even if something is wrong with the server, presumably we would find that out as soon as we try to do a 'get' from the cache, so what's the point in checking it early,

      The comment above the ping line is not helpful as to why it's needed.

      While this is only a very small amount of time, for sites that use Redis (hosted on a separate machine) which is probably very common, that time is taken on every single request, so deleting this line would seem like a nice easy performance gain. Unless there is a good reason why it's actually needed, in which case the comment should probably be improved!

      Attachments

        Activity

          People

            quen Sam Marshall
            quen Sam Marshall
            Brendan Heywood Brendan Heywood
            Victor Déniz Falcón Victor Déniz Falcón
            Angelia Dela Cruz Angelia Dela Cruz
            Matteo Scaramuccia, David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
            Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:
              28/Nov/22

              Time Tracking

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