Last modified: 2014-01-29 06:18:02 UTC
The fix to bug 55986 has a problem, which is that incr loses the expiration date. This is because Redis does not preserve it on set (http://redis.io/commands/expire). So until we can use the built-in Redis INCR, we need to re-set the expiration.
It should work (with a very small time error) to check it with http://redis.io/commands/ttl then reset it after the increment.
We should really just not serialize integers, which could avoid all these problems without a bunch of round trips.
Agreed. It might cause some breakage with existing values, though.
(In reply to comment #2) > We should really just not serialize integers, which could avoid all these > problems without a bunch of round trips. We should really file this as an upstream bug.
(In reply to comment #4) > We should really file this as an upstream bug. To where, phpredis? I'm fine with that if it makes sense, but we should first think a bit what the "right" thing is. Note that strings are also serialized currently ("s:3:\"123\";"). So if you simply serialize neither, you can no longer preserve the type info: gettype( "3" ) vs. gettype( 3 ) That might be fine for some applications, but not always. Or you could let numbers be the only thing not serialized in SERIALIZER_PHP mode, but I'm not 100% positive that's best either. And of course, if you don't serialize it all, it solves this, but if I understand the phpredis code, your objects and arrays are now just "Object" and "Array".
(In reply to comment #5) > That might be fine for some applications, but not always. Or you could let > numbers be the only thing not serialized in SERIALIZER_PHP mode, but I'm not > 100% positive that's best either. Even if it is for us, it may or may not work upstream (and existing values are still an issue).
Change 96801 had a related patch set uploaded by Aaron Schulz: Fixes to RedisBagOStuff https://gerrit.wikimedia.org/r/96801
Change 96801 merged by jenkins-bot: Fixes to RedisBagOStuff https://gerrit.wikimedia.org/r/96801
