Skip to content

rate-limiting: Redis keys have wrong TTL when sync_rate > 0 #14995

Description

@jmadureira

Is there an existing issue for this?

  • I have searched the existing issues

Kong version ($ kong version)

Reproduced on Kong 3.9.x. The same code is on master (3091de0).

Current Behavior

With policy = redis and sync_rate > 0, the rate-limiting plugin leaves counter keys in Redis with a bigger TTL than expected.

This happens because timestamp.get_timestamps() returns period start times in milliseconds, but the Redis EXPIREAT operation expects seconds.

Expected Behavior

Every key written by the rate-limiting plugin to the Redis cache should have a TTL expressed in seconds.

Steps To Reproduce

  1. Configure the rate limiting plugin with policy redis, sync_rate > 1 and with a limit of 60 requests per minute.
  2. Make some requests to Kong
  3. Check that the TTLs of the keys in Redis are "theoretically" 1000x bigger then expected.

The following spec also recreates the issue.

  describe("redis with sync rate > 0 and key missing in redis at sync time", function()
    local EXPIRATION = require "kong.plugins.rate-limiting.expiration"

    local redis
    local conf = {
      route_id = uuid(),
      service_id = uuid(),
      redis_host = helpers.redis_host,
      redis_port = helpers.redis_port,
      redis_database = 0,
      sync_rate = 1,
    }

    before_each(function()
      local red = require "resty.redis"
      redis = assert(red:new())
      redis:set_timeout(1000)
      assert(redis:connect(conf.redis_host, conf.redis_port))
      redis:flushall()
    end)

    after_each(function()
      redis:close()
    end)

    for _, period in ipairs { "minute", "hour" } do
      it("sets a TTL no longer than one " .. period .. " when recreating the key", function()
        local identifier = uuid()
        -- same unit the handler uses (milliseconds)
        local current_timestamp = ngx.time() * 1000
        local periods = timestamp.get_timestamps(current_timestamp)
        local cache_key = get_local_key(conf, identifier, period, periods[period])

        -- populates the local cache, including the key's expire_at
        assert(policies.redis.usage(conf, identifier, period, current_timestamp))

        -- simulate the key expiring in redis before the pending delta is synced
        assert(redis:del(cache_key))

        assert(policies.redis.increment(conf, { [period] = 10 }, identifier, current_timestamp, 1))
        assert(policies.redis.increment(conf, { [period] = 10 }, identifier, current_timestamp, 1))

        -- give time to the async sync to recreate the key
        ngx.sleep(1 + conf.sync_rate)

        local ttl = assert(redis:ttl(cache_key))
        assert(ttl > 0 and ttl <= EXPIRATION[period],
          "expected TTL in (0, " .. EXPIRATION[period] .. "], got " .. ttl)
      end)
    end
  end)

Anything else?

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions