Tweet caching may be too agressive

I propose that the module has stopped working entirely. I've dug around in the code and I can see that the __construct() method is being called, but I can't see that any of the other functions are being called in the twitter_block.class.php file. At this point in time, the block is configured correctly on my site, but nothing is happening when the page is loaded.

I suspect this happened when I upgraded to the latest version, from March 24, 2012.

Update: The block works, but as long as I use something other than "getTweetsFrom". That seems to be what changed on March 24. All options for "Choose the type of Tweets" work file except "Tweets sent from the Twitter User"

Please check the watchdog logs. Any extra info?

Hi @fcjversc and Lets troubleshoot this:

1. Clear your browser's cache and try to load the page again.
2. Check to be sure there are actually new results for the search you are using. Try https://twitter.com/#!/search/
3. IIRC, we did recently implement some tweet caching, so try clearing your drupal cache at admin/settings/performance then reload the page. If this works, please report back here and we will look into it more.
4. Go to admin/reports/dblog. Are there any messages for twitter block? Report them here.

Thanks!

@joelrichard please don't hijack this issue. If the module does not work for you at all, open a new issue and describe your problem with very specific details.

I had this issue - tweets not being displayed at all after module upgrade. Clearing all caches via drush caused the latest ones to display OK. However, caching was not enabled at the time - Cache pages for anonymous users and Cache blocks both unchecked on admin/config/development/performance.

Thanks @cboyden.
It's possible the new(ish) caching of tweets #1096134: Tweet limit may be too agressive. Need to investigate further to rule this out.

Perhaps configuration options could help with this too. Some sites may want to cache the tweets, some may not? Not really sure w/o looking more closely.

Perhaps we could just drop the tweet caching and let the page cache handle it?

Agreed @cweagans. Perhaps the code introduced in #1096134: Tweet limit to cache tweets should be surrounded with a conditional instead of being removed entirely tho. Something should allow themers and the like to refresh the page multiple times without running into the twitter limit.

Actually, is that even relevant since we adopted the RESTful API?

To do:

• Check the RESTful API for limits.
• Optionally add a config for caching.
• Add a UI for setting the config (and the debug mode)

Pretty sure it still is relevant with the RESTful API (see https://dev.twitter.com/docs/rate-limiting#rest).

Currently tweets are cached until a general cache wipe occurs. This means that tweets will never be updated on a site which never requires the cache to be cleared. I think this amount of caching is unnecessary, especially as twitter is meant for frequent updates.

The Twitter REST API permitted 150 unauthenticated calls to per hour, so the cache can be stored for a minimum of 24 seconds to stay within the limit. That said, I'd probably stick to once a minute, just to be safe.

The attached patch makes the cached Twitter search results expire every 60 seconds. It also does some cleanup to the entire twitter_block_load_tweets function.

+++ b/twitter_block.moduleundefined
@@ -184,40 +184,44 @@ function twitter_block_block_view($delta) {
+  // Use a cached copy of the Twitter search results when possible

If a search doesn't return data, then every page load will trigger new API calls to Twitter, which could put us over the rate limit. I'm thinking that we should just go ahead and cache the output regardless. Perhaps we could lower the cache lifetime to the bare minimum that wouldn't put us over the rate limit.

Also consider that this same site may have other modules that use the Twitter API, so we need to be cautious with how many API calls we're making.

+++ b/twitter_block.moduleundefined
@@ -184,40 +184,44 @@ function twitter_block_block_view($delta) {
-      watchdog('Twitter Block', 'Recieved an unexpected reply from Twitter. ' .
-        'Perhaps just a fail whale?<br/>' .
-        'URL: url_query<br />' .
-        'response', array('url_query' => $twitter->url_query, 'response' => print_r($response, TRUE)),
-        WATCHDOG_NOTICE);

Any particular reason for making this change?

+++ b/twitter_block.moduleundefined
@@ -184,40 +184,44 @@ function twitter_block_block_view($delta) {
+    // Cache the search results for an hour

Either this comment needs to change, or the code needs to change.

Hi

I'm not sold on the idea that tweets should be cached using:

cache_set($cid, $results, $cache_bin, CACHE_TEMPORARY);

As previously mentioned in #9, this implies that the cache will never be refreshed unless a "general cache wipe" occurs, ie a call to cache_clear_all(). This is fine during dev, but i dont see a reason for ever calling this function once a site is live, hence the tweets will never be refreshed!

Surely the sensible way to do it is by using a cron hook to clear cached tweets that are beyond a certain age (ideally configurable via the CMS)?

I'm going to write a cron hook to do this myself, i'll aim to post on here when i'm done.

Cheers

After a few hellish hours of cache research and fiddling, i've discovered the following:

Garbage collection is managed (to some degree) by core in the system_cron() hook. This gathers a list of cache tables used throughout the site, and iteratively calls:

cache_clear_all(NULL, $table);

The problem here is that passing NULL in this way to cache_clear_all() does not seem to clear entries "aggressively" enough. I ran out of time trying to discover whether this is a problem in core, or whether this "works as designed" but just does not handle our use case properly.

Now take a look at the code in the drupal_flush_all_caches() function - which i noticed DOES refresh the tweets. The following call is used:

cache_clear_all('*', $table, TRUE);

'*' is a wildcard for the CID, which causes ALL cache entries to be flushed - ie far more aggressive than passing NULL

Therefore, i've managed to fix the problem by creating the following cron hook:

function mymodule_cron(){
	cache_clear_all('twitter_block_', 'cache', TRUE);
}

This forces all twitter_block_ entries to be cleared, and refreshed the next time the twitter block is requested. Perhaps this is a bit of a sledgehammer, but it only clears entries specific to twitter_block, and ultimately fixes the problem.

Any chance of getting this cron hook added to the twitter_block module?

Cheers

Let's let the user/admins decide. We'll add an option to the block configuration so folks can choose more aggressive caching if that's their thing. I'd like to match the block cache options in hook_block_info, but I have not looked closely enough to see if that's practical yet.

We really don't need to be caching this data at all. The between the block cache and the page cache, we should be okay to just retrieve the data, render it, and call it good.

A patch to remove the tweet caching.

I haven't tested it, but that looks right.

Committed #16 to 7.x-1.x.

Backport of #16.

Committed #19 to 6.x-1.x.