-
Notifications
You must be signed in to change notification settings - Fork 382
Description
There are a few places where DB writes are being performed during plugin initialization or when performing normal frontend requests to the site:
Lines 404 to 420 in f1af0ab
| /* | |
| * Broadcast plugin updates. | |
| * Note that AMP_Options_Manager::get_option( 'version', '0.0' ) cannot be used because | |
| * version was new option added, and in that case default would never be used for a site | |
| * upgrading from a version prior to 1.0. So this is why get_option() is currently used. | |
| */ | |
| $options = get_option( AMP_Options_Manager::OPTION_NAME, array() ); | |
| $old_version = isset( $options['version'] ) ? $options['version'] : '0.0'; | |
| if ( AMP__VERSION !== $old_version ) { | |
| /** | |
| * Triggers when after amp_init when the plugin version has updated. | |
| * | |
| * @param string $old_version Old version. | |
| */ | |
| do_action( 'amp_plugin_update', $old_version ); | |
| AMP_Options_Manager::update_option( 'version', AMP__VERSION ); | |
| } |
(Removed class-amp-story-templates.php instance since code removed in #4315.)
(Removed AMP_Theme_Support::check_for_cache_misses() example since removed in #4178.)
These can potentially cause race conditions where concurrent requests both cause an update to be done at the same time. It's also not great for performance to have DB writes being performed in response to frontend requests.
I suggest that each of these cases be refactored with wp_schedule_single_event() which can then be used to perform the writes during WP Cron, and thus avoid the race condition.
It's true that scheduling a WP Cron event is itself a DB write, but at least this a more proper way to do it. Otherwise, there could be a repeating task every X minutes to see if the above tasks need to be performed.