search

Automattic / WPCOM-Legacy-Redirector

WordPress plugin for handling large volumes of legacy redirects in a scalable manner.
19 stars 15 forks source link

Performance issue caused by wpcom_vip_add_role_caps #77

Closed mslinnea closed 3 years ago

mslinnea commented 4 years ago

We noticed a performance issue with this plugin on VIP Go. The function register_redirect_custom_capability is hooked in to run at init, which calls wpcom_vip_add_role_caps on the VIP environment. This should only be called when the capabilities change, per the VIP documentation at https://wpvip.com/documentation/vip-go/custom-user-roles/

GaryJones commented 4 years ago

Yikes, you're right! Thanks for reporting, @mslinnea.

Originally added here by @mikeyarce before the class got moved to it's own file.

Code is now here.

mslinnea commented 3 years ago

We fixed this performance issue by adding this to theme code: remove_action( 'init', [ 'WPCOM_Legacy_Redirector', 'register_redirect_custom_capability' ] );

As for fixing this in the plugin, I think this should only run during plugin activation or during a plugin upgrade routine. It's important to make sure this only runs in a limited and controlled manner.

GaryJones commented 3 years ago

When plugins are activated by code on VIP, there is no plugin activation or upgrade routine.

But, we should be able to protect against by using the approach at https://docs.wpvip.com/how-tos/customize-user-roles/.

mslinnea commented 3 years ago

We experienced an issue with the customize-user-roles code suggestion. The issue is that when the version is changed, all page requests will attempt to update the option at the same time. On a site with a lot of traffic, this caused a brief site outage as the database experienced an overload. We moved this update option code to a CLI command so that it only runs once. Alternatively, it could be limited to running only in the admin, which most likely wouldn't create enough requests to overload the database.

GaryJones commented 3 years ago

@mslinnea Please see #94 - the hook is now admin_init, the capability registration incorporates a version check, and we've added some unit tests to have more confidence that everything is working.