Optimize bulk plugin updates

[jipmoors]

When selecting more then one plugins to update the following things are done:

• Maintenance mode on (if -a- plugin is active)

• Per plugin:

1. Get plugin info
2. Download package
3. Unpack package
4. Install package

• Maintenance mode off

Scenario:
10 plugins require updates.
Only the last one is active = requires maintenance mode to be enabled.
Server might not have the best connection to WordPress.org or other plugin resource.

This means that the site will not be available during:

• Downloading of all plugins
• Unpacking of all plugins
• Installing of all plugins

This means at least (10*download request) + (10*unpacking) + (10*installing) = 30 actions.

Not including optional plugin info request * 10.
Also not including plugins that need to do delta's on tables or other upgrading scripts.

Though only one plugin actually required the site to not be available, which would be (1*installing) = 1 action.

Ideal flow:

• Download all packages
• Unpack downloaded packages

• Determine per plugin if maintenance is required
• Install plugin
• Disable maintenance if next plugin is not active

• Finally: disable maintenance if enabled
• Remove all unpacked packages and downloads

I can think of a performance argument to include the unpacking of packages in the maintenance mode because it might be a strain on the server, but functionally I don't think it should be.

As far as I can see the only file that this is effectively handled in is class-wp-upgrader.php.

[jipmoors]

Separated Bulk from Upgrader

[jipmoors]

The added patch separates the functionality of the Bulk upgrades from the Upgrader.
Because of how the skin is build up, the feedback has been modified a bit.

All the plugins that are upgraded are shown, after that you wait until all actions have been completed.
It would be preferable that these steps have their own output: downloading packages, unpacking downloads, installing plugins. I don't have any experience on dealing with these language/translation additions so guidance would be welcome.

All functionality has been maintained, only distributed differently.

Tested:

• 1 plugin updated (update-core.php)
• 3 plugins updated (update-core.php)
• 1 active plugin, 2 inactive plugins updated (update-core.php)
• 3 active plugins updated (update-core.php)
• 1 plugin updated (plugins.php)
• 1 theme updated
• 3 themes updated
• language updates

[DrewAPicture]

Looking at 34676.1.patch​, the initial things in terms of documentation that jump out at me are:

• The version should 4.5.0 (we aren’t going to introduce something like this in a point release)
• All of the property DocBlocks should have descriptions
• The phpDoc tag description don’t need to be aligned
• Several parameters don’t have descriptions at all
• There should only be one class per file

[jipmoors]

One class per file, docs improved

[jrf]

Related: #36618

[ocean90]

@jipmoors Looking at 34676.2.class-seperation-and-docs.diff​: In src/wp-admin/update-core.php you changed include_once( ABSPATH . 'wp-admin/includes/class-wp-upgrader.php' ); to require_once( ABSPATH . 'wp-admin/includes/class-language-pack-upgrader.php' ); but Language_Pack_Upgrader extends WP_Upgrader. Was this a mistake or how did you solve this?

[jipmoors]

@ocean90 it seems I totally screwed that patch up. I must say I'm not so fluent at SVN at this point ;)
There are many files missing and that logic is broken as you pointed out.

Base functionality is all in the first patch, the second one is just separating files.
I don't mind re-doing the patch if it has any added value.
Otherwise happy to refresh/improve the first patch and document as required.

[jipmoors]

Revisiting this ticket again I think I also would like to rethink the architectural choice made and probably chance the approach a bit.

[ocean90]

Replying to jipmoors:

@ocean90 it seems I totally screwed that patch up. I must say I'm not so fluent at SVN at this point ;)

No worries. :) The split is now handled in #36618.

[karmatosed]

Noting this needs a patch refresh, once we have the patch then it would be good to see what is being suggested. Showing some screenshots would also be really good, to help people see.

[karmatosed]

The design team would still love to give feedback on this. Could we have screenshots of the refreshed patch please?

[airathalitov]

This is how I see it:

Logic for updating one plugin:

• Download plugin
• Unpack plugin

• If plugin is active {
• Turn on maintenance mode
• Install plugin
• Disable maintenance mode }

• Else if plugin is not active {
• Install plugin (without maintenance mode) }

• Delete old files

Logic for updating multiple plugins:

• Download all plugins
• Unpack all plugins
• Check status of each plugin (active|inactive)
• Group plugins into 2 groups by status (active|inactive)
• Install all inactive plugins (without enabling maintenance mode)
• Enable maintenance mode
• Install all active plugins
• Disable maintenance mode
• Delete old files

This algorithm should work faster. And you do not need to constantly switch the maintenance mode.

[jipmoors]

Before - 1 update (active)

After - 1 update (active)

Before - 2 updates (1 active, 1 inactive)

This will activate maintenance mode surrounding the entire process.

After - 2 updates (1 active, 1 inactive)

This will only activate maintenance mode for the active plugin.

[jipmoors]

Refreshed patch

[jipmoors]

The refreshed patch improved the visual representation of the bulk updates.

It does not implement any changes for languages (which the first patch did).
So it just does it for plugins and themes.

[airathalitov]

@jipmoors
Could you test your patch on this example sequence and show a screenshot:

• active plugin
• disabled plugin
• active plugin
• disabled plugin
• active plugin

[jipmoors]

Running the suggested situation, I noticed that the identifier for which plugin is being installed is based on the list of plugins as they appear on the plugin page (and selector on the upgrade page).

I think it would be clearer to make it sequential.

See the screenshots:

[jipmoors]

Also note that for an upgrade of plugins/themes on a multisite network-admin, maintenance mode will be enabled around all updates - as it is implemented in the current situation.

[jipmoors]

@airathalitov I have not separated the deletion of old files from the flow, as it would require separating more logic from bulk and non-bulk installs. This is something I wanted to reduce for this first improvement.

Disk usage

Though this point has me realizing that this new method of walking through the flow has a potential disk-space issue, which was not as prominent before.

As all downloads and unzips are taking place beforehand, the space requires has increased to the total amount of all plugins being upgraded. Before this was only the size of the biggest plugin.

As a site running at near full disk is at risk from being unavailable for many other reasons, I don't think it should be an instant blocker. This concern should be taken into account in determining if it's acceptable for merge.

[ Linking this comment for consult from the hosting-providers slack channel ]

[jipmoors]

As this improvement has not been raised as an actual problem, I want to stress that not having this improvement would be a perfectly acceptable outcome for me as well.

I personally think the user experience will benefit and having less maintenance mode seconds is an improvement. I also want to have a full consideration, especially on the implications I am unaware of or you have better understand off. So I'm still very much behind this improvement :)

[airathalitov]

The patch is working great. Thanks!

I thought about disk usage before.

Did you delete zip archives of plugins after unpacking each of them?

• Download all plugins
• For each plugin in list {
• Unpack package
• Delete zip archive }
• ... (other steps from https://core.trac.wordpress.org/ticket/34676#comment:17 )

For example If we have 10 plugins to update It would take not bigger than 100MB. I think it's not so big and everything should be good.

[jipmoors]

Deleting the zip is also working the same as before, when it's a local file it will not be removed - otherwise the unpacking mechanism removes the file when it's done.

[jipmoors]

Extended patch 2 with correct plugin counter (1/5...5/5)

[jipmoors]

The one thing I have not been able to fix is that the unpacking feedback shows this:

Unpacking the update…
Unpacking the update…
Unpacking the update…
Unpacking the update…
Unpacking the update…

Which is just one line per plugin being unpacked.
As this feedback is given at a place where the context is missing totally, I don't see an easy way of improving on this.

[airathalitov]

Maybe should it contain package name?

Unpacking plugin-name.x.x.x.zip…

instead of

Unpacking the update…

Example:

Unpacking gutenberg.5.7.0.zip…
Unpacking hello-dolly.1.6.zip…
Unpacking wordpress-seo.11.2.1.zip…
...

It looks more informative.

[jipmoors]

Currently only the tmp file is available, which is not matching the zip or plugin name. So I decided not to add that to it.

[airathalitov]

Or another variant

Unpacking the update… (1/5)
Unpacking the update… (2/5)
Unpacking the update… (3/5)
Unpacking the update… (4/5)
Unpacking the update… (5/5)

And you don't need to change the translation strings.

[jipmoors]

Extended patch 3: unpack (1/x)

[jipmoors]

Was able to implement the Unpacking ... (1/5) suggestion, had to add a translation string which holds the counting as this could be placed differently for other languages.

Just realized I forgot translator comments, will add those.

[jipmoors]

Extended patch 4: Added translator comments

[desrosj]

Looks like the latest patch needs a refresh and this still needs unit tests.

With 5.3 Beta 1 in 3 days, I am going to punt this to 5.4. This should probably land early in the cycle to allow for some soak time and iteration.

[davidbaumwald]

This ticket was discussed during the #core bug scrub today. Given this still needs a refresh and the early tag, this is being moved to Future Release. If any committer feels this can be moved back up or can own it during a specific cycle, feel free to update the milestone.

[bookdude13]

Refreshed patch 5

[bookdude13]

Numbering wrong before, corrected

[bookdude13]

Update page, some active plugins and some not.