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

Optimise $DB->replace_all_text()

    XMLWordPrintable

Details

    • MOODLE_34_STABLE
    • MOODLE_33_STABLE, MOODLE_34_STABLE
    • MDL-60976-master
    • Hide

      Use the replace tool to validate that the replace still works after applying the patch:

      1. Add some text to a label in a course. e.g. 'http://linkedin.com'
      2. Use the /tool/replace/ to search for 'http://' and replace it with 'https://'
      3. Check that the label in the course now contains 'https://linkedin.com'

      It's worth noting that unit tests already cover the replace_all_test() function well.

       

      It would also be good to do a test with all dbs that support replace_all_text() with a larger dataset. Possibly by doing an http -> https migration testing that performance is improved after this patch is applied. This works really well on postgres but have not tested mysql or MS Sql.

      Show
      Use the replace tool to validate that the replace still works after applying the patch: Add some text to a label in a course. e.g. ' http://linkedin.com' Use the /tool/replace/ to search for ' http://'  and replace it with ' https://' Check that the label in the course now contains ' https://linkedin.com' It's worth noting that unit tests already cover the replace_all_test() function well.   It would also be good to do a test with all dbs that support replace_all_text() with a larger dataset. Possibly by doing an http -> https migration testing that performance is improved after this patch is applied. This works really well on postgres but have not tested mysql or MS Sql.

    Description

      On Postgres the $DB->replace_all_text() function is very inefficient as it updates every tuple even if the REPLACE doesn't actually change anything. This is slow and results in big spikes in DB disk usage when using tool_replace.

      The attached patch only updates fields if they match the search pattern.

      Attachments

        Issue Links

          Activity

            People

              matt.clarkson Matt Clarkson
              matt.clarkson Matt Clarkson
              Dan Marsden Dan Marsden
              Damyon Wiese Damyon Wiese
              Jake Dallimore Jake Dallimore
              David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                15/Jan/18