Hi,

We have a lot of "s3fs_file" select queries.
Because we are using redis (or memcache), using standard drupal caching tables for this info, can be a big optimalisation.

Is this a good idea? I am going to digg into the code, and can use any help.

We can do this caching in _read_cache() in S3fsStreamWrapper.inc

Comments

Jurgen8en’s picture

Issue summary: View changes
Jurgen8en’s picture

Like this?
Where to clear the cache?
cache_clear_all('s3fs:uri:'.$uri, 'cache');

   protected function _read_cache($uri) {
   	 $this->_debug("_read_cache($uri) called.", TRUE);
   	 
   	 // Since public:///blah.jpg and public://blah.jpg refer to the same file
   	 // (a file named blah.jpg at the root of the file system), we'll sometimes
   	 // receive files with a /// in their URI. This messes with our caching
   	 // scheme, though, so we need to remove the extra /.
   	 if (strpos($uri, 'public:///') === 0) {
   	 	 $uri = preg_replace('^public://[/]+^', 'public://', $uri);
   	 }
   	 else if (strpos($uri, 'private:///') === 0) {
   	 	 $uri = preg_replace('^private://[/]+^', 'private://', $uri);
   	 }
   	 
   	 
   	 
   	 //Jurgen8en
   	 $cid = 's3fs:uri:'.$uri;
   	 if ($cached = cache_get($cid, 'cache')) {
   	 	 $record = $cached->data;
   	 }
   	 else {
   	 	 // Cache miss. Avoid a stampede
   	 	 if (!lock_acquire($cid, 1)) {
   	 	 	 // Another request is building the variable cache.
   	 	 	 // Wait, then re-run this function.
   	 	 	 lock_wait($cid);
   	 	 	 $record = _read_cache($uri);
   	 	 }
   	 	 else {
   	 	 	 /**********FUNCTION without cache***********/    
   	 	 	 
   	 	 	 $record = db_select('s3fs_file', 's')
   	 	 	 ->fields('s')
   	 	 	 ->condition('uri', $uri, '=')
   	 	 	 ->execute()
   	 	 	 ->fetchAssoc();
   	 	 	 
   	 	 	 /**********FUNCTION without cache***********/
   	 	 	 cache_set($cid, $record, 'cache', CACHE_TEMPORARY);
   	 	 	 lock_release($cid);
   	 	 }
   	 }
   	 
   	 return $record ? $record : FALSE;
   }
coredumperror’s picture

I find it fairly amusing that what you're doing here is caching a cache, since s3fs_file is a local cache of the file metadata in the S3 bucket. But I agree that this is probably a good idea for folks using faster caching a mechanism than the DB.

This feature will require a little more code than than what you've shown here, though, since the metadata cache refresh mechanism will need to also clear the drupal cache. I'll go ahead and implement that, and push it up to git in a bit.

  • coredumperror committed 839a546 on 7.x-2.x
    Issue #2512838: Added Drupal caching to the s3fs metadata cache.
    
    This...
coredumperror’s picture

Alright, I pushed a change that I'm hoping will be the correct implementation for this caching. Please download the latest dev release to try it out, and let me know the results, good or bad.

coredumperror’s picture

Status: Active » Needs review
Jurgen8en’s picture

I am going to test later.

  • coredumperror committed 839a546 on 8.x-2.x
    Issue #2512838: Added Drupal caching to the s3fs metadata cache.
    
    This...
coredumperror’s picture

Status: Needs review » Closed (fixed)

Since this has been in the code since June, and no one's complained about it, I'm assuming it at least didn't cause any problems. So I'm going to close this issue as "fixed". If you feel that this issue should be re-opened, feel free to do so.

5n00py’s picture

I found that commit #08e6bc0630d3ce7b6dac65712b3c3dccaca78a29 removing this caching mechanism.

Can someone tell about performance changes with/without this patch?