fb-instant-articles icon indicating copy to clipboard operation
fb-instant-articles copied to clipboard

Performance consideration

Open sboisvert opened this issue 7 years ago • 1 comments

Steps required to reproduce the problem

  1. Have a large sites with hundreds of concurrent views
  2. Update the instant-articles-option-publishing option (which deletes all the meta_keys that act as caches)

Actual Result

What happens is that is_empty_after_transformation()'s cache is now empty making it do update_post_meta(), this would be fine if it was on save or something but should_submit_post() is run inside inject_link_rel() which is run on the front end.

Here is a filter we've added to "fix" this:

//Instant articles performance improvement. Do not allow post_meta writes on the front end.
add_filter( 'instant_articles_should_submit_post', 'wpcom_vip_only_update_instant_article_cache_on_backend', 10,2 );
function wpcom_vip_only_update_instant_article_cache_on_backend( $should_show, $IA_object ){
	//Copied from has_warnings_after_transformation()
	$cache = get_post_meta( $IA_object->get_the_id(), '_has_warnings_after_transformation', true );
	if ( $cache && $cache === 'yes' ) {
		return false;
	}

	// copied from is_empty_after_transformation()
	$cache = get_post_meta( $IA_object->get_the_id(), '_is_empty_after_transformation', true );
	if ( $cache && $cache === 'yes' ) {
		return false;
	}

	//We want to block DB writes on the front end from the has_warnings_after_transformation function that stores the cache we checked above.
	if ( is_admin() ){
	    return true;
    }
    //If we're not on the admin we don't want the other functions to run.
    return false;
}

I'm not sure how you want to address this, either as a won't fix or as a toggle inside the plugin that turns this on (using this filter and then having a manually run WP-CLI command or something that updates the posts one at a time would be the best way to achieve this [kicking off a cron job to do it one at a time would also be fine])


Version Info

  • Plugin version: 4.0
  • WordPress version: trunk
  • PHP version: 7.0

sboisvert avatar Oct 13 '17 17:10 sboisvert

Hi @sboisvert!

That's a very reasonable concern.

In your solution, you filter so only the admins would trigger a cache fill? In such case, all other users will cache miss until an admin loads each article, leading to fairly slow read queries to run for these users. Am I missing something?

Another solution we should consider is running the code on the FE only for the Facebook Crawler, by looking at the user agent. Besides debugging purposes, the only requests that needs to have the ia:markup_url meta are the ones by the Crawler.

What do you think? Would you consider creating a Pull-Request for that?

Thank you so much for your help! Diego

diegoquinteiro avatar Nov 13 '17 02:11 diegoquinteiro

This issue has been marked stale because it has been open for 30 days with no activity. If there is no activity within 7 days, it will be closed. This is an automation to keep issues manageable and actionable and is not a comment on the quality of this issue nor on the work done so far. Closed issues are still valuable to the project and are available to be searched.

github-actions[bot] avatar Oct 17 '22 00:10 github-actions[bot]