Why Redis keys are not expiring?

Question

I have checked these questions but they did not help me to fix my issue. I am using Redis as a key value store for Rate Limiting in my Spring REST application using spring-data-redis library. I test with huge load. In that I use the following code to store a key and I am setting the expire time as well. Most of the time the key expires as expected. But some times the key is not expiring!

code snippet

RedisAtomicInteger counter = counter = new RedisAtomicInteger("mykey");
counter.expire(1, TimeUnit.MINUTES);

I checked the availability of the keys using redis-cli tool

keys *

and

ttl keyname

redis.conf having default values.

Any suggestions ?

Edit 1:

Full code:

The function is in an Aspect

public synchronized Object checkLimit(ProceedingJoinPoint joinPoint) throws Exception, Throwable {

        boolean isKeyAvailable = false;
        List<String> keysList = new ArrayList<>();

        Object[] obj = joinPoint.getArgs();
        String randomKey = (String) obj[1];
        int randomLimit = (Integer) obj[2];

        // for RedisTemplate it is already loaded as 

        // @Autowired
        // private RedisTemplate template; 

        // in this class
        Set<String> redisKeys = template.keys(randomKey+"_"randomLimit+"*");
        Iterator<String> it = redisKeys.iterator();
        while (it.hasNext()) {
               String data = it.next();
               keysList.add(data);
        }

        if (keysList.size() > 0) {
            isKeyAvailable = keysList.get(0).contains(randomKey + "_" + randomLimit);
        }

        RedisAtomicInteger counter = null;
        // if the key is not there
        if (!isKeyAvailable) {
              long expiryTimeStamp = 0;
              int timePeriodInMintes = 1;
              expiryTimeStamp = new Date(System.currentTimeMillis() + timePeriodInMintes * 60 * 1000).getTime();
              counter = new RedisAtomicInteger(randomKey+ "_"+ randomLimit + "_" + expiryTimeStamp,template.getConnectionFactory());
              counter.incrementAndGet();
              counter.expire(timePeriodInMintes, TimeUnit.MINUTES);
              break;

        } else {

              String[] keys = keysList.get(0).split("_");
              String rLimit = keys[1];

              counter = new RedisAtomicInteger(keysList.get(0), template.getConnectionFactory());
              int count = counter.get();
              // If count exceeds throw error
              if (count != 0 && count >= Integer.parseInt(rLimit)) {
                    throw new Exception("Error");
               }  
               else {
                    counter.incrementAndGet();
              }
      }
        return joinPoint.proceed();
    }

when these lines run

RedisAtomicInteger counter = counter = new RedisAtomicInteger("mykey"); counter.expire(1, TimeUnit.MINUTES);

I can see

75672562.380127 [0 10.0.3.133:65462] "KEYS" "mykey_1000*"
75672562.384267 [0 10.0.3.133:65462] "GET" "mykey_1000_1475672621787"
75672562.388856 [0 10.0.3.133:65462] "SET" "mykey_1000_1475672621787" "0"
75672562.391867 [0 10.0.3.133:65462] "INCRBY" "mykey_1000_1475672621787" "1"
75672562.395922 [0 10.0.3.133:65462] "PEXPIRE" "mykey_1000_1475672621787" "60000"
...
75672562.691723 [0 10.0.3.133:65462] "KEYS" "mykey_1000*"
75672562.695562 [0 10.0.3.133:65462] "GET" "mykey_1000_1475672621787"
75672562.695855 [0 10.0.3.133:65462] "GET" "mykey_1000_1475672621787"
75672562.696139 [0 10.0.3.133:65462] "INCRBY" "mykey_1000_1475672621787" "1" 

in Redis log, when I "MONITOR" it in

Answer 1

Edit: Now with the updated code I believe your methodology is fundamentally flawed aside from what you're reporting.

The way you've implemented it you require running KEYS in production - this is bad. As you scale out you will be causing a growing, and unnecessary, system blocking load on the server. As every bit of documentation on it says, do not use keys in production. Note that encoding the expiration time in the key name gives you no benefit. If you made that part of the key name a the creation timestamp, or even a random number nothing would change. Indeed, if you removed that bit, nothing would change.

A more sane route would instead be to use a keyname which is not time-dependent. The use of expiration handles that function for you. Let us call your rate-limited thing a "session". Your key name sans the timestamp is the "session ID". By setting an expiration of 60s on it, it will no longer be available at the 61s mark. So you can safely increment and compare the result to your limit without needing to know the current time or expiry time. All you need is a static key name and an appropriate expiration set on it.

If you INCR a non-existing key, Redis will return "1" meaning it created the key and incremented it in a single step/call. so basically the logic goes like this:

1. create "session" ID
2. increment counter using ID
3. compare result to limit
   1. if count == 1, set expiration to 60s
   2. id count > limit, reject

Step 3.1 is important. A count of 1 means this is a new key in Redis, and you want to set your expiration on it. Anything else means the expiration should already have been set. If you set it in 3.2 you will break the process because it will preserve the counter for more than 60s.

With this you don't need to have dynamic key names based on expiration time, and thus don't need to use keys to find out if there is an existing "session" for the rate-limited object. It also makes your code much simpler and predictable, as well as reduce round trips to Redis - meaning it will be lower load on Redis and perform better. As to how to do that w/the client library you're using I can't say because I'm not that familiar with it. But the basic sequence should be translatable to it as it is fairly basic and simple.

What you haven't shown, however, is anything to support the assertion that the expiration isn't happening. All you've done is show that Redis is indeed being told to and setting an expiration. In order to support your claim you need to show that the key does not expire. Which means you need to show retrieval of the key after the expiration time, and that the counter was not "reset" by being recreated after the expiration. One way you can see the expiration is happening is to use keyspace notifications. With that you will be able to see Redis saying a key was expired.

Where this process will fail a bit is if you do multiple windows for rate-limiting, or if you have a much larger window (ie. 10 minutes) in which case sorted sets might be a more sane option to prevent front-loading of requests - if desired. But as your example is written, the above will work just fine.