RedisCached storage is not durable on first request

[azgabur]

Describe the bug
When RedisCached storage is set in Limitador CR and the limitador pod gets restarted, the very first http request to the Gateway is never rate limited (no 429 status code) even if it should be. Subsequent http requests behave normally (are rate limited)

To Reproduce
Have redis running and storage-secret created as specified in docs https://github.com/Kuadrant/limitador-operator/blob/main/doc/storage.md

1. Set Limitador CR to use RedisCached and wait until ready. The options are not needed as even with defaults the bug appears.

apiVersion: limitador.kuadrant.io/v1alpha1
kind: Limitador
metadata:
  name: limitador
spec:
  storage:
    redis-cached:
      configSecretRef:
        name: storage-secret
      options:
        batch-size: 1
        flush-period: 1
        max-cached: 1

2. Apply Gateway, HttpRoute, and RLP. The RLP window time should be large enough so there is time for pod restart

apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: gw
  namespace: kuadrant
spec:
  gatewayClassName: istio
  listeners:
    - allowedRoutes:
        namespaces:
          from: Same
      hostname: '*.example.com'
      name: api
      port: 80
      protocol: HTTP
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: route
  namespace: kuadrant
spec:
  hostnames:
    - hostname.example.com
  parentRefs:
    - group: gateway.networking.k8s.io
      kind: Gateway
      name: gw
  rules:
    - backendRefs:
        - group: ''
          kind: Service
          name: httpbin
          namespace: kuadrant
          port: 8080
          weight: 1
      matches:
        - path:
            type: PathPrefix
            value: /
---
apiVersion: kuadrant.io/v1
kind: RateLimitPolicy
metadata:
  name: limit
  namespace: kuadrant
spec:
  limits:
    basic:
      rates:
        - limit: 5
          window: 600s
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: route

3. Perform enough http requests to trigger rate limiting. Last request should be rate limited and return 429

for i in {1..6}; do curl -I hostname.example.com; done

3. Restart limitador pod and wait until finish:

oc rollout -n kuadrant-system restart deployment limitador-limitador
oc rollout -n kuadrant-system status deployment limitador-limitador

5. Do 3 requests for the endpoint

for i in {1..3}; do curl -I hostname.example.com; done

First will return 200 and subsequent ones are correctly rate limited with 429

Expected behavior
The first http request after limitador pod restart should be rate limited and return 429

Additional context
Happens on newest nightly kuadrant builds and also last stable release of Kuadrant.

[azgabur]

I also noticed unexpected 200 after pod restart when using only Redis storage, could be connected to this bug. I noticed it happens when running tests with cached and non-cached versions of Redis storage in close time frame.

[thomasmaas]

Analysis: this appears to be working as designed, not a bug

The redis-cached backend explicitly trades accuracy for speed. The design comment states:

We can go over limits, but the amount can be configured by tuning the constants below.

And check_and_update documents the tradeoff directly:

This function trades accuracy for speed.

What happens on pod restart

1. First request arrives, counter is not in local cache → cache miss
2. load_from_authority_asap(counter, 0) creates a temporary local counter with value 0 and from_authority: false
3. Since value is 0, the request is not limited → allowed through
4. The from_authority: false flag triggers a priority flush to sync with Redis on the next batcher cycle
5. Second request onwards: cache is populated with the real Redis value → correctly limited

This is the intended behavior for the cached backend. The first request on a cold cache miss is allowed through by design.

The cold-start case is a different category of inaccuracy

The accuracy tradeoff makes sense for steady-state multi-instance drift (two pods incrementing locally, batching to Redis, accepting small bounded overshoot). But on a cold start, the local cache has zero knowledge while Redis may hold a counter at max_value. Assuming 0 can overshoot by up to the entire limit window — that's unbounded, unlike the small drift the design intends.

However, there's no way to distinguish "cache miss because I just restarted" from "cache miss because this counter is genuinely new" without querying Redis. Adding a synchronous Redis fetch on cache miss would fix the cold-start case, but it would also change steady-state behavior for all first-seen counters — every new client, endpoint, or window rotation would incur a Redis round-trip that the current design intentionally avoids. That changes the contract of the cached backend, not just the restart path.

This is a design question for the Limitador maintainers: is the cold-start overshoot worth addressing, and if so, is the cost of a Redis GET on every first-seen counter acceptable? Or should this be handled outside the library (e.g., an operator-level readiness gate that delays traffic until the first batcher sync completes)?

Plain Redis variant

The plain redis (async) backend fetches from Redis on every request via MGET, so it should not have this issue. The commenter's observation about plain Redis may be test interference from running cached and non-cached tests in close succession.

cc @alexsnaps

[azgabur]

If this is documented as a feature I am fine with it. But wouldn't it be reasonable to load all counters to cache on first start to address the durability bug/tradeoff? Or regular refreshes even without request to the gateway as options.flush-period suggests.

Also this bug happens also with non-cached "redis" storage provider in global rate limiting scenario. After the max-counter is reached across two clusters one more request is required (which passes) before rate-limit applies. See https://github.com/Kuadrant/testsuite/blob/12fceadbf620bdafc0396ab19b0795067d3de314/testsuite/tests/multicluster/global_rate_limiting/test_global_rate_limit.py#L53