Make out-of-sequence plugin upgrades consistent in CLI upgrader & web upgrader

Bug #1614805 reported by Aaron Wells on 2016-08-19
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Medium
Aaron Wells
15.04
Medium
Aaron Wells
15.10
Medium
Aaron Wells
16.04
Medium
Aaron Wells
16.10
Medium
Aaron Wells

Bug Description

In Bug 1614298 there were some differences of behavior between the CLI upgrader and the web upgrader, due to how they handled the out-of-sequence installation of the module/webservices plugin. This plugin is installed in the core lib/db/upgrade.php file, by a call to plugin_upgrade(), in order to make sure it gets installed instead of leaving it as an optional user-initiated installation.

The CLI upgrader operates by running "check_upgrades()" at the beginning, which then looks at every component and plugin in Mahara to see which ones need to be upgraded. The return data includes the "installed" version number (retrieved from the database) and the "new" version number (from the version.php files on the filesystem). It then passes this data to "upgrade_mahara($data)", which loops over the array and runs the installer for each component.

As a result, the webservices plugin was listed as needing upgrading, and then it got installed during the "core" upgrade step by the call to "upgrade_plugin()", and again by upgrade_mahara() when it looped through the listed plugins needing installation. The version number passed to the upgrade function by upgrade_mahara() was based on the initial call to check_upgrades() at the start of the script, causing the same upgrade block in the plugin's db/upgrade.php script to be executed twice.

The AJAX upgrader didn't have this problem, because it re-checks the status of each plugin before running the upgrade function for that plugin. First the parent script calls "check_plugins()" to find all the plugins that need to be upgraded. Then it sends a request to upgrade.json.php for each upgrade component. And then upgrade.json.php calls check_plugins() again. It does this in order to retrieve the full data about the plugin; but it has the side effect of preventing the double-upgrading of plugins that were upgraded or installed out of order by core.

It'd be best if both installation routes performed the same.

Aaron Wells (u-aaronw) wrote :

To test:

1. Install Mahara 15.04
2. Check out the gerrit patch for the improved upgrader (in 16.10dev)
3. Run the command-line upgrader.

Expected result: No errors

If you're using my Mahara DevTools ( https://github.com/agwells/mahara-devtools ) you can just use its "maharaupgrade.sh" to do the CLI upgrader.

Otherwise, you can run:

  sudo -u www-data php htdocs/admin/cli/upgrade.php

(I find it's best to run the CLI upgrader sudo'ed to the www-data user so that the permissions in the dataroot don't get messed up.)

Reviewed: https://reviews.mahara.org/6872
Committed: https://git.mahara.org/mahara/mahara/commit/0e1704c1365bf372ee4a56cebe0785f130dc139e
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 0e1704c1365bf372ee4a56cebe0785f130dc139e
Author: Aaron Wells <email address hidden>
Date: Fri Aug 19 15:04:27 2016 +1200

CLI upgrader, better handling of out-of-sequence plugin upgrades

Bug 1614805. When a plugin is installed "out of sequence" by a
call to plugin_upgrade in the core lib/db/upgrade.php file, the
AJAX-based web upgrader handles it gracefully, because it does
check_upgrade() before upgrading each plugin, detects the change
in status, and doesn't run the upgrade for that plugin a second
time.

The CLI upgrader, on the other hand, uses the same cached data
from check_upgrades() all the way through, causing it to re-run
upgrades for such plugins.

This patch makes it behave the same as the AJAX plugin, running
check_upgrade() again immediately before each component gets
upgraded. This does cause some redundancy in code execution,
but it shouldn't be enough to cause a noticeable performance hit.

Change-Id: Id5c431fc9e636df2cab05d22e6cc424271ce9f3d
behatnotneeded: Covered by existing tests

Mahara Bot (dev-mahara) wrote :

Patch for "15.10_STABLE" branch: https://reviews.mahara.org/6888

Mahara Bot (dev-mahara) wrote :

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/6889

Reviewed: https://reviews.mahara.org/6889
Committed: https://git.mahara.org/mahara/mahara/commit/af5e7ae39d5237aaa35be03ead005ee50db6080e
Submitter: Son Nguyen (<email address hidden>)
Branch: 15.04_STABLE

commit af5e7ae39d5237aaa35be03ead005ee50db6080e
Author: Aaron Wells <email address hidden>
Date: Fri Aug 19 15:04:27 2016 +1200

CLI upgrader, better handling of out-of-sequence plugin upgrades

Bug 1614805. When a plugin is installed "out of sequence" by a
call to plugin_upgrade in the core lib/db/upgrade.php file, the
AJAX-based web upgrader handles it gracefully, because it does
check_upgrade() before upgrading each plugin, detects the change
in status, and doesn't run the upgrade for that plugin a second
time.

The CLI upgrader, on the other hand, uses the same cached data
from check_upgrades() all the way through, causing it to re-run
upgrades for such plugins.

This patch makes it behave the same as the AJAX plugin, running
check_upgrade() again immediately before each component gets
upgraded. This does cause some redundancy in code execution,
but it shouldn't be enough to cause a noticeable performance hit.

Change-Id: Id5c431fc9e636df2cab05d22e6cc424271ce9f3d
behatnotneeded: Covered by existing tests
(cherry picked from commit 0e1704c1365bf372ee4a56cebe0785f130dc139e)

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/6888
Committed: https://git.mahara.org/mahara/mahara/commit/3a6fcfb94d0bcda49a3ca97524f4e5360a807495
Submitter: Son Nguyen (<email address hidden>)
Branch: 15.10_STABLE

commit 3a6fcfb94d0bcda49a3ca97524f4e5360a807495
Author: Aaron Wells <email address hidden>
Date: Fri Aug 19 15:04:27 2016 +1200

CLI upgrader, better handling of out-of-sequence plugin upgrades

Bug 1614805. When a plugin is installed "out of sequence" by a
call to plugin_upgrade in the core lib/db/upgrade.php file, the
AJAX-based web upgrader handles it gracefully, because it does
check_upgrade() before upgrading each plugin, detects the change
in status, and doesn't run the upgrade for that plugin a second
time.

The CLI upgrader, on the other hand, uses the same cached data
from check_upgrades() all the way through, causing it to re-run
upgrades for such plugins.

This patch makes it behave the same as the AJAX plugin, running
check_upgrade() again immediately before each component gets
upgraded. This does cause some redundancy in code execution,
but it shouldn't be enough to cause a noticeable performance hit.

Change-Id: Id5c431fc9e636df2cab05d22e6cc424271ce9f3d
behatnotneeded: Covered by existing tests
(cherry picked from commit 0e1704c1365bf372ee4a56cebe0785f130dc139e)

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/6887
Committed: https://git.mahara.org/mahara/mahara/commit/4c5b9d1138ef2230d204773beedd0ad200bbd9e7
Submitter: Son Nguyen (<email address hidden>)
Branch: 16.04_STABLE

commit 4c5b9d1138ef2230d204773beedd0ad200bbd9e7
Author: Aaron Wells <email address hidden>
Date: Fri Aug 19 15:04:27 2016 +1200

CLI upgrader, better handling of out-of-sequence plugin upgrades

Bug 1614805. When a plugin is installed "out of sequence" by a
call to plugin_upgrade in the core lib/db/upgrade.php file, the
AJAX-based web upgrader handles it gracefully, because it does
check_upgrade() before upgrading each plugin, detects the change
in status, and doesn't run the upgrade for that plugin a second
time.

The CLI upgrader, on the other hand, uses the same cached data
from check_upgrades() all the way through, causing it to re-run
upgrades for such plugins.

This patch makes it behave the same as the AJAX plugin, running
check_upgrade() again immediately before each component gets
upgraded. This does cause some redundancy in code execution,
but it shouldn't be enough to cause a noticeable performance hit.

Change-Id: Id5c431fc9e636df2cab05d22e6cc424271ce9f3d
behatnotneeded: Covered by existing tests
(cherry picked from commit 0e1704c1365bf372ee4a56cebe0785f130dc139e)

Robert Lyon (robertl-9) on 2016-10-21
Changed in mahara:
milestone: 16.10.0 → none
Robert Lyon (robertl-9) on 2016-10-25
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers