Retain hashes for current AND current-1 versions by samhotchkiss · Pull Request #6637 · Automattic/jetpack

samhotchkiss · 2017-03-13T15:37:10Z

This addresses an issue on large sites which have multiple web servers accessing a single database. In that scenario, if updates are applied as a roll out rather than simultaneously, it will cause excessive database load as the versions "fight" over this option.

jeherve

Minor nitpicking to make our Code Sniffer happy.

jeherve · 2017-03-13T16:07:56Z

class.jetpack.php

+
 		$file_data_option[ JETPACK__VERSION ][ $key ] = $data;
+
+		if( count( $file_data_option ) > 2 ) {


phpcs will complain about the missing space after if here.

jeherve · 2017-03-13T16:08:08Z

class.jetpack.php

+			krsort( $file_data_option );
+			foreach ( $file_data_option as $version => $values ) {
+				$count++;
+				if( $count > 2 && JETPACK__VERSION != $version ) {


Missing space before if

samhotchkiss · 2017-03-13T16:28:09Z

Fixed, thanks @jeherve

thingalon

Mostly looks good, but the sort order may break at .10 releases.

thingalon · 2017-03-15T03:51:49Z

class.jetpack.php

+
+		if ( count( $file_data_option ) > 2 ) {
+			$count = 0;
+			krsort( $file_data_option );


If you sort version numbers as strings, this will break at 4.10.0, which will sort as < 4.9.

I suggest something like:

$versions = array_reverse( usort( array_keys( $file_data_options ), 'version_compare' ) );

We have never released a '.10', and will never do so-- Jetpack doesn't practice semver

briancolinger

I tested these changes by executing the following in wp shell.
return Jetpack::get_module( 'protect' );

var_dump()'ing the $file_data_option array before and after the version pruning shows the expected results.

Jetpack don't speak semver

simonwheatley · 2017-03-27T18:58:47Z

I've applied this patch to a copy of Jetpack 4.6, then to a copy of Jetpack 4.7.1, and upgraded from one to the other. All seems fine.

Here's the output for the jetpack_files_data option after the upgrade, I can see an entry for 4.6 and an entry for 4.7.1:

jetpack_file_data.4.7.1.txt

simonwheatley · 2017-03-27T19:57:50Z

I just ran a light load test on a site with a two web server cluster, one running a copy of 4.6 with this patch and another running a copy of Jetpack 4.7.1 with this patch. The rate of UPDATE and INSERT commands during an upgrade is less with the patch, but still present.

Tested using the methodology above.

Graph below. The first group of spikes (19:26 to 19:38) is an upgrade during a load test where the patch is not applied. The second group of spikes (19:42 to 19:48) is the same, but with the patch applied.

I have not tested Jetpack generally for issues with this patch, I've purely concentrated on MySQL commands during an upgrade, as described above.

Here's the UPDATE and INSERT queries I noted were still running on every page load while there were inconsistent versions of Jetpack running:

UPDATE `wp_options`
SET `option_value` = 'a:1:{s:3:\"4.6\";a:39:{s:18:\"after-the-deadline\";s:3:\"1.1\";s:8:\"carousel\";s:3:\"1.5\";s:8:\"comments\";s:3:\"1.4\";s:12:\"contact-form\";s:3:\"1.3\";s:20:\"custom-content-types\";s:3:\"3.1\";s:10:\"custom-css\";s:3:\"1.7\";s:21:\"enhanced-distribution\";s:3:\"1.2\";s:16:\"google-analytics\";s:3:\"4.5\";s:19:\"gravatar-hovercards\";s:3:\"1.1\";s:15:\"infinite-scroll\";s:3:\"2.0\";s:8:\"json-api\";s:3:\"1.9\";s:5:\"latex\";s:3:\"1.1\";s:5:\"likes\";s:3:\"2.2\";s:6:\"manage\";s:3:\"3.4\";s:8:\"markdown\";s:3:\"2.8\";s:9:\"minileven\";s:3:\"1.8\";s:7:\"monitor\";s:3:\"2.6\";s:5:\"notes\";s:3:\"1.9\";s:10:\"omnisearch\";s:3:\"2.3\";s:6:\"photon\";s:3:\"2.0\";s:13:\"post-by-email\";s:3:\"2.0\";s:7:\"protect\";s:3:\"3.4\";s:9:\"publicize\";s:3:\"2.0\";s:13:\"related-posts\";s:3:\"2.9\";s:9:\"seo-tools\";s:3:\"4.4\";s:10:\"sharedaddy\";s:3:\"1.1\";s:10:\"shortcodes\";s:3:\"1.1\";s:10:\"shortlinks\";s:3:\"1.1\";s:8:\"sitemaps\";s:3:\"3.9\";s:3:\"sso\";s:3:\"2.6\";s:5:\"stats\";s:3:\"1.1\";s:13:\"subscriptions\";s:3:\"1.2\";s:13:\"tiled-gallery\";s:3:\"2.1\";s:10:\"vaultpress\";s:5:\"0:1.2\";s:18:\"verification-tools\";s:3:\"3.0\";s:10:\"videopress\";s:3:\"2.5\";s:17:\"widget-visibility\";s:3:\"2.4\";s:7:\"widgets\";s:3:\"1.2\";s:7:\"wordads\";s:5:\"4.5.0\";}}'
WHERE `option_name` = 'jetpack_available_modules'	
INSERT INTO wp_options (option_name, option_value, autoload)
VALUES ('jpsq_sync-1490643930.608226-222028-1', 'a:5:{i:0;s:14:\"updated_option\";i:1;a:3:{i:0;s:25:\"jetpack_available_modules\";i:1;a:1:{s:5:\"4.7.1\";a:39:{s:18:\"after-the-deadline\";s:3:\"1.1\";s:8:\"carousel\";s:3:\"1.5\";s:8:\"comments\";s:3:\"1.4\";s:12:\"contact-form\";s:3:\"1.3\";s:20:\"custom-content-types\";s:3:\"3.1\";s:10:\"custom-css\";s:3:\"1.7\";s:21:\"enhanced-distribution\";s:3:\"1.2\";s:16:\"google-analytics\";s:3:\"4.5\";s:19:\"gravatar-hovercards\";s:3:\"1.1\";s:15:\"infinite-scroll\";s:3:\"2.0\";s:8:\"json-api\";s:3:\"1.9\";s:5:\"latex\";s:3:\"1.1\";s:5:\"likes\";s:3:\"2.2\";s:6:\"manage\";s:3:\"3.4\";s:8:\"markdown\";s:3:\"2.8\";s:9:\"minileven\";s:3:\"1.8\";s:7:\"monitor\";s:3:\"2.6\";s:5:\"notes\";s:3:\"1.9\";s:10:\"omnisearch\";s:3:\"2.3\";s:6:\"photon\";s:3:\"2.0\";s:13:\"post-by-email\";s:3:\"2.0\";s:7:\"protect\";s:3:\"3.4\";s:9:\"publicize\";s:3:\"2.0\";s:13:\"related-posts\";s:3:\"2.9\";s:9:\"seo-tools\";s:3:\"4.4\";s:10:\"sharedaddy\";s:3:\"1.1\";s:10:\"shortcodes\";s:3:\"1.1\";s:10:\"shortlinks\";s:3:\"1.1\";s:8:\"sitemaps\";s:3:\"3.9\";s:3:\"sso\";s:3:\"2.6\";s:5:\"stats\";s:3:\"1.1\";s:13:\"subscriptions\";s:3:\"1.2\";s:13:\"tiled-gallery\";s:3:\"2.1\";s:10:\"vaultpress\";s:5:\"0:1.2\";s:18:\"verification-tools\";s:3:\"3.0\";s:10:\"videopress\";s:3:\"2.5\";s:17:\"widget-visibility\";s:3:\"2.4\";s:7:\"widgets\";s:3:\"1.2\";s:7:\"wordads\";s:5:\"4.5.0\";}}i:2;a:1:{s:3:\"4.6\";a:39:{s:18:\"after-the-deadline\";s:3:\"1.1\";s:8:\"carousel\";s:3:\"1.5\";s:8:\"comments\";s:3:\"1.4\";s:12:\"contact-form\";s:3:\"1.3\";s:20:\"custom-content-types\";s:3:\"3.1\";s:10:\"custom-css\";s:3:\"1.7\";s:21:\"enhanced-distribution\";s:3:\"1.2\";s:16:\"google-analytics\";s:3:\"4.5\";s:19:\"gravatar-hovercards\";s:3:\"1.1\";s:15:\"infinite-scroll\";s:3:\"2.0\";s:8:\"json-api\";s:3:\"1.9\";s:5:\"latex\";s:3:\"1.1\";s:5:\"likes\";s:3:\"2.2\";s:6:\"manage\";s:3:\"3.4\";s:8:\"markdown\";s:3:\"2.8\";s:9:\"minileven\";s:3:\"1.8\";s:7:\"monitor\";s:3:\"2.6\";s:5:\"notes\";s:3:\"1.9\";s:10:\"omnisearch\";s:3:\"2.3\";s:6:\"photon\";s:3:\"2.0\";s:13:\"post-by-email\";s:3:\"2.0\";s:7:\"protect\";s:3:\"3.4\";s:9:\"publicize\";s:3:\"2.0\";s:13:\"related-posts\";s:3:\"2.9\";s:9:\"seo-tools\";s:3:\"4.4\";s:10:\"sharedaddy\";s:3:\"1.1\";s:10:\"shortcodes\";s:3:\"1.1\";s:10:\"shortlinks\";s:3:\"1.1\";s:8:\"sitemaps\";s:3:\"3.9\";s:3:\"sso\";s:3:\"2.6\";s:5:\"stats\";s:3:\"1.1\";s:13:\"subscriptions\";s:3:\"1.2\";s:13:\"tiled-gallery\";s:3:\"2.1\";s:10:\"vaultpress\";s:5:\"0:1.2\";s:18:\"verification-tools\";s:3:\"3.0\";s:10:\"videopress\";s:3:\"2.5\";s:17:\"widget-visibility\";s:3:\"2.4\";s:7:\"widgets\";s:3:\"1.2\";s:7:\"wordads\";s:5:\"4.5.0\";}}}i:2;i:111;i:3;d:1490643930.60822200775146484375;i:4;b:0;}','no')

@see

@see 40e115c#commitcomment-21523982

@SInCE

* Readme: remove old release and add skeleton for 4.8. * Changelog: add #6572 * Changelog: add #6567 * Changelog: add #6542 * Changelog: add #6527 * Changelog: add #6508 * Changelog: add #6478 * Changelog: add #6477 * Changelog: add #6249 * Update stable version and remove old version from readme. * Changelog: add 4.7.1 to changelog. * Readme: add new contributor. * Sync: update docblock @SInCE version. Related: #6053 * Changelog: add release post. * changelog: add #6053 * Changelog: add #6413 * Changelog: add #6482 * Changelog: add #6584 * Changelog add #6603 * Changelog: add #6606 * Changelog: add #6611 * Changelog: add #6635 * Changelog: add #6639 * Changelog: add #6684 * Changelog: add #6710 * Changelog: add #6711 * Changelog: add #5461 * Testing list: update Settings UI feedback prompt. Props @MichaelArestad * Changelog: add #6789 * Changelog: add #6778 * Changelog: add #6777 * Changelog: add #6775 * Changelog: add #6755 * Changelog: add #6731 * Changelog: add #6721 * Changelog: add #6705 * Changelog: add #6702 * Changelog: add #6671 * Changelog: add #6637 * Changelog: add #6582 * Changelog: add #6566 * Changelog: add #6555 * Changelog: add #6529 * Changelog: add #6344 * Changelog: add #5763 * Changelog: add #5503 * Changelog: update #6637 changelog. @see 40e115c#commitcomment-21523982 * Changelog: add #6699 * Changelog: add #6632 * Changelog: add #6769 * Changelog: add #6707 * Changelog: add #6590

Retain hashes for current AND current-1 versions

f29d65f

samhotchkiss requested a review from simonwheatley March 13, 2017 15:37

samhotchkiss added [Focus] Performance [Pri] High [Status] Needs Review This PR is ready for review. labels Mar 13, 2017

Extra safety

1d5a426

jeherve added General Bug When a feature is broken and / or not performing as intended labels Mar 13, 2017

jeherve reviewed Mar 13, 2017

View reviewed changes

Add spaces after ifs

2d80265

thingalon previously requested changes Mar 15, 2017

View reviewed changes

thingalon added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 16, 2017

samhotchkiss added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 27, 2017

jeherve added this to the 4.8 milestone Mar 27, 2017

briancolinger approved these changes Mar 27, 2017

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 27, 2017

thingalon merged commit 11f24af into master Mar 28, 2017

thingalon deleted the fix/load-issues-on-server-clusters branch March 28, 2017 00:22

matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 28, 2017

jeherve added [Status] Needs Changelog and removed [Status] Needs Changelog labels Mar 28, 2017

jeherve added a commit that referenced this pull request Mar 28, 2017

Changelog: add #6637

40e115c

jeherve added a commit that referenced this pull request Mar 28, 2017

Changelog: update #6637 changelog.

8adbafd

@see 40e115c#commitcomment-21523982

jeherve mentioned this pull request May 1, 2017

General: make sure get_file_data is always unserialized #7097

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retain hashes for current AND current-1 versions#6637

Retain hashes for current AND current-1 versions#6637
thingalon merged 3 commits intomasterfrom
fix/load-issues-on-server-clusters

samhotchkiss commented Mar 13, 2017

Uh oh!

jeherve left a comment

Uh oh!

jeherve Mar 13, 2017

Uh oh!

jeherve Mar 13, 2017

Uh oh!

samhotchkiss commented Mar 13, 2017

Uh oh!

thingalon left a comment

Uh oh!

thingalon Mar 15, 2017 •

edited

Loading

Uh oh!

samhotchkiss Mar 27, 2017

Uh oh!

briancolinger left a comment

Uh oh!

simonwheatley commented Mar 27, 2017

Uh oh!

simonwheatley commented Mar 27, 2017 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants


		$file_data_option[ JETPACK__VERSION ][ $key ] = $data;

		if( count( $file_data_option ) > 2 ) {

Conversation

samhotchkiss commented Mar 13, 2017

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve Mar 13, 2017

Choose a reason for hiding this comment

Uh oh!

jeherve Mar 13, 2017

Choose a reason for hiding this comment

Uh oh!

samhotchkiss commented Mar 13, 2017

Uh oh!

thingalon left a comment

Choose a reason for hiding this comment

Uh oh!

thingalon Mar 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samhotchkiss Mar 27, 2017

Choose a reason for hiding this comment

Uh oh!

briancolinger left a comment

Choose a reason for hiding this comment

Uh oh!

simonwheatley commented Mar 27, 2017

Uh oh!

simonwheatley commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thingalon Mar 15, 2017 •

edited

Loading

simonwheatley commented Mar 27, 2017 •

edited

Loading