Make WordPress Core

Opened 14 years ago

Last modified 4 weeks ago

#11585 assigned enhancement

WordPress should cache failed feed fetches so as to avoid overloading feed sources

Reported by: denis-de-bernardy's profile Denis-de-Bernardy Owned by: pbearne's profile pbearne
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: Feeds Keywords: has-patch
Focuses: performance Cc:

Description

Following up on #11219, which fixed the cause of that particular error, but not the one that related to caching of feed errors.

When SimplePie fails to fetch or parse a feed, it should not hammer whichever server the feed came from on every page load. This is bad for two reasons: it disturbs the originating server, and it tremendously slows down page loads when the feed is in an RSS widget on the front end.

Instead, we should cache the error for a reasonably long amount of time (15 minutes? An hour? More?) and bypass SimplePie until that duration expires.

Attachments (2)

11585.diff (3.6 KB) - added by peterchester 11 years ago.
11585.2.diff (6.9 KB) - added by peterchester 11 years ago.

Download all attachments as: .zip

Change History (13)

#1 @hakre
14 years ago

Idea makes sense. I know from one site I use simplepie on and where I do logging that feels fail from time to time like out of nothing. Tend to a smaller timespan like 15 minutes.

#2 @mtdewvirus
14 years ago

What about caching the feed items twice? Once with a long cache expiration and once with the current expiration. If the current cache expires and the feed isn't available, repopulate from the long expiration cache and set a shorter expiration to check if the feed is back up. If the feed is available, update both caches. This way, a visitor doesn't need to see an error and will still see content.

#3 @nacin
14 years ago

  • Milestone changed from 3.0 to Future Release

Needs a patch.

#4 @peterchester
11 years ago

Taking a look at this...

@peterchester
11 years ago

#5 @peterchester
11 years ago

  • Keywords has-patch added; needs-patch removed

Ok, I think I have it. But honestly, this part of the code is pretty hard to follow. I've tested locally with working and broken feed urls but it would be great if this could be tested more thoroughly with feeds that are throttled and / or temporarily disabled.

Some questions that came up in the process:

  • Should we be displaying errors in the RSS widget on the frontend when a feed is not working or should we just hide the widget?
  • The error message for a bad feed appears in the RSS admin when you first enter it. But after saving it again, the error disappears because the feed is cached (empty). Should we be effectively caching the error in the widget admin?

#6 @peterchester
11 years ago

Just confirmed that the error message for a failed feed only showed up on the first save before this fix so that's not a new issue if it's an issue at all.

I'm also adding a new patch with a solution for separating the cache timing from the error cache timing. Initial testing on that came out positively. To test cache timings use the filters 'wp_feed_cache_transient_lifetime' (the existing one) vs. 'wp_feed_error_cache_transient_lifetime' (the new one)

To test, add the following to your functions.php file to easily see the timeouts if you load a functional feed vs. a broken one.

add_filter( 'wp_feed_cache_transient_lifetime', 'my_feed_cache_transient_lifetime' );
function my_feed_cache_transient_lifetime() {
	return 30; // 30 second cache life time for successful content
}

add_filter( 'wp_feed_error_cache_transient_lifetime', 'my_feed_error_cache_transient_lifetime' );
function my_feed_error_cache_transient_lifetime() {
	return 5; // 5 second cache life time for errors.
}

I tested broken feeds by entering a url in the RSS widget that i knew was totally bunk and one that was loading xml that made Simplepie sad vs. my control of a feed on a live site that was working fine.

#7 @nacin
10 years ago

  • Component changed from Performance to Feeds
  • Focuses performance added

#8 @chriscct7
8 years ago

  • Keywords needs-refresh added

#10 @pbearne
4 weeks ago

  • Keywords needs-refresh removed
  • Owner set to pbearne
  • Status changed from new to assigned

refreshed patch

This ticket was mentioned in PR #6168 on WordPress/wordpress-develop by @pbearne.


4 weeks ago
#11

The update introduces a new feature in the SimplePie class: error caching. This will allow the SimplePie to hold errors for a specified time period before they're deleted. The changes also include a call of 'wp_feed_error_cache_transient_lifetime' filter within the new error caching function in feed.php. Error messages in widgets.php are now wrapped in an HTML "error" class for better styling control.

Trac ticket: 11585

Note: See TracTickets for help on using tickets.