Bug #78120 for CHI-Driver-Redis: Store subroutine expiration is not working properly

Sat Jun 30 12:21:53 2012 mess1214 [...] gmail.com - Ticket created

Subject:	Store subroutine expiration is not working properly
Date:	Sat, 30 Jun 2012 11:21:41 -0500
To:	bug-CHI-Driver-Redis [...] rt.cpan.org
From:	James M <mess1214 [...] gmail.com>

I noticed that when creating a CHI object using the Redis driver, with expires_on_backend set to 1, all cache entries were expiring immediately. I was setting expirations for keys using the expires_in argument. When debugging, I noticed that in CHI::Driver::Redis::store(), the argument you have named $expires_at (and expecting an epoch time) is actually the expires_in value (in seconds). I believe that variable should be renamed to $expires_in, and then instead of converting to $secs, you just need $self->redis->expire($realkey, $expires_in);

Tue Apr 23 10:36:59 2013 chico.lopes [...] gmail.com - Correspondence added

From:

chico.lopes [...] gmail.com

And here is the diff file for James M.'s suggestion, tested and working.

Subject:

fix_expires_on_backend.patch

--- Redis.pm.old 2013-04-23 15:20:58.000000000 +0100 +++ Redis.pm 2013-04-23 15:21:28.000000000 +0100 @@ -111,7 +111,7 @@ } sub store { - my ($self, $key, $data, $expires_at, $options) = @_; + my ($self, $key, $data, $expires_in) = @_; return unless $self->_verify_redis_connection; @@ -126,9 +126,8 @@ } $self->redis->set($realkey => $data); - if(defined($expires_at)) { - my $secs = $expires_at - time; - $self->redis->expire($realkey, $secs); + if(defined($expires_in)) { + $self->redis->expire($realkey, $expires_in); } }

Tue Apr 23 10:37:00 2013 The RT System itself - Status changed from 'new' to 'open'

Mon Jun 23 13:35:56 2014 ether [...] cpan.org - Taken

Mon Jun 23 14:14:48 2014 ether [...] cpan.org - Correspondence added

On 2013-04-23 07:36:59, chico.lopes@gmail.com wrote: Show quoted text

> And here is the diff file for James M.'s suggestion, tested and working.

The code in the github repository has changed since the 0.04 release - in particular, the code pertaining to the key expiration has been commented out entirely. Can you give this a test and let me know if it works better? If so, I'll release it to cpan as-is as version 0.05. that's https://github.com/gphat/chi-driver-redis

Fri Aug 01 16:15:28 2014 ianburrell [...] gmail.com - Stolen from ETHER

Fri Aug 01 17:24:45 2014 ianburrell [...] gmail.com - Correspondence added

On Mon Jun 23 14:14:48 2014, ETHER wrote: Show quoted text

> On 2013-04-23 07:36:59, chico.lopes@gmail.com wrote:

> > And here is the diff file for James M.'s suggestion, tested and > > working.

> > The code in the github repository has changed since the 0.04 release - > in particular, the code pertaining to the key expiration has been > commented out entirely. Can you give this a test and let me know if > it works better? If so, I'll release it to cpan as-is as version > 0.05. > > that's https://github.com/gphat/chi-driver-redis

I have taken over maintaining CHI::Driver::Redis. The new repo is https://github.com/rentrak/chi-driver-redis. I resolved the problem with version 0.04 in Git being different from what was released as 0.04. One of the new changes is to change to storing values in Redis hash for namespace instead of storing in each key. This means expiration can't be stored on each key which is why that part was commented out. I'm inclined to revert using hashes and use Redis expiration.

Fri Aug 01 20:09:11 2014 ether [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #78120] Store subroutine expiration is not working properly
Date:	Fri, 1 Aug 2014 17:08:59 -0700
To:	Ian Burrell via RT <bug-CHI-Driver-Redis [...] rt.cpan.org>
From:	Karen Etheridge <ether [...] cpan.org>

On Fri, Aug 01, 2014 at 05:24:45PM -0400, Ian Burrell via RT wrote: Show quoted text

> I have taken over maintaining CHI::Driver::Redis. The new repo is https://github.com/rentrak/chi-driver-redis. I resolved the problem with version 0.04 in Git being different from what was released as 0.04.

Awesome; welcome! Show quoted text

> One of the new changes is to change to storing values in Redis hash for namespace instead of storing in each key. This means expiration can't be stored on each key which is why that part was commented out. I'm inclined to revert using hashes and use Redis expiration.

Yes, I fear that any existing user will face problems when updating, unless a script was provided to also migrate all the data over -- which means that they need to be aware that their system must come down while the upgrade takes place.

Mon Aug 04 17:08:33 2014 ianburrell [...] gmail.com - Correspondence added

Fixed in 0.06.

Mon Aug 04 17:08:34 2014 ianburrell [...] gmail.com - Status changed from 'open' to 'resolved'