WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 10 months ago

#34676 new enhancement

Optimize bulk plugin updates

Reported by: jipmoors Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4
Component: Upgrade/Install Keywords: has-patch ux-feedback needs-unit-tests shiny-updates
Focuses: performance Cc:

Description

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.

Attachments (2)

34676.1.patch (38.6 KB) - added by jipmoors 2 years ago.
Separated Bulk from Upgrader
34676.2.class-seperation-and-docs.diff (136.2 KB) - added by jipmoors 2 years ago.
One class per file, docs improved

Download all attachments as: .zip

Change History (14)

@jipmoors
2 years ago

Separated Bulk from Upgrader

#1 @jipmoors
2 years ago

  • Keywords has-patch ux-feedback needs-unit-tests added

#2 @jipmoors
2 years ago

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

#3 @DrewAPicture
2 years ago

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
2 years ago

One class per file, docs improved

#4 @dd32
2 years ago

  • Component changed from Plugins to Upgrade/Install

#5 @obenland
2 years ago

  • Keywords shiny-updates added

#6 @jrf
2 years ago

Related: #36618

Last edited 2 years ago by ocean90 (previous) (diff)

#7 @ocean90
2 years ago

@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?

#8 follow-up: @jipmoors
2 years ago

@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.

#9 @jipmoors
2 years ago

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

#10 in reply to: ↑ 8 @ocean90
2 years ago

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.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


10 months ago

#12 @karmatosed
10 months ago

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.

Note: See TracTickets for help on using tickets.