Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis keys not expiring #417

Closed
Tracked by #536
cdaguerre opened this issue Dec 13, 2023 · 15 comments · Fixed by #536
Closed
Tracked by #536

Redis keys not expiring #417

cdaguerre opened this issue Dec 13, 2023 · 15 comments · Fixed by #536

Comments

@cdaguerre
Copy link

The Redis storage provider does not set key TTL correctly (using Redis 6.2). Or is the behavior below expected?
Capture d’écran 2023-12-13 à 14 00 38

@darkweak
Copy link
Owner

Hey @cdaguerre thats definitively a bug 😄

@darkweak
Copy link
Owner

@cdaguerre can you paste an the request you're sending and the responses (the one with the store and the one with the age/ttl) ?

@darkweak
Copy link
Owner

darkweak commented Mar 5, 2024

@cdaguerre should be fixed in the latest release.

@K0tBegemot
Copy link

I have this problem too. In standlone mode with latest release, /flush method does not clear the cache contents

@darkweak
Copy link
Owner

@K0tBegemot do you have a curl request?

@HobMartin
Copy link
Contributor

Same problem to me. I use Redis cluster and when I send PURGE request, keys in Redis still exist.
After trying to reproduce it, I believe I understand why it happened.

Keys and Stale was store in one cluster slot, but IDXs stores in different and when PURGE request call DeleteMany method I've got panic: multi key command with different key slots are not allowed.

How fix that?

@darkweak
Copy link
Owner

darkweak commented Apr 2, 2024

@HobMartin Can you paste your configuration, and a curl request to reproduce please? 🙏

@HobMartin
Copy link
Contributor

HobMartin commented Apr 2, 2024

@darkweak Of course.
My Caddy config:

(redis_cache) {
  redis {
    configuration {
      InitAddress clustercfg.xxxxx.yyyyy.use1.cache.amazonaws.com:6379 // here cluster config endpoint
      TLSConfig
      Username "username"
      Password "password"
    }
  }
}
{
  cache {

    api {
      basepath /_cache
      souin
    }
    import redis_cache

    stale 60s
    ttl 120s

    key {
      disable_body
      disable_method
      headers Content-Type Authorization
      hide
    }
  }
}

And curl request

curl -x PURGE https://cache-testing.test.com/_cache/souin/cache-testing

I use Souin Api endpoint with name of site to remove caching for a specific site. BTW,

curl -x PURGE https://cache-testing.test.com/_cache/souin/

doesn't work too.
And PURGE by surrogate key, didn't clear as well

FYI, I specify TLSConfig, because without it Caddy failed when starting.

As I know, issue with cross slot could be fixed by using https://redis.io/docs/reference/cluster-spec/#hash-tags in key name. And It will be great for me, because I can specify hashtag by different site. But I cannot customize cache key in Souin 😢

@HobMartin
Copy link
Contributor

@darkweak any updates?

@darkweak
Copy link
Owner

@HobMartin sorry for the delay. I opened a PR that concerns the redis storage #489. I will try to take your issue in it. About the hashtags, it it really mandatory? Maybe this redis/rueidis#177 (reply in thread) could solve that instead (DoMulti usage).

@darkweak
Copy link
Owner

@HobMartin I pushed a new commit that may support the redis hashtags. You can enable it using the HashTag directive in the redis configuration.

{
    order cache before rewrite
    cache {
        api {
            souin
        }
        ttl 1000s
    }
}

:4443
respond "Hello World!"

route /redis-configuration/* {
    cache {
        redis {
            configuration {
                HashTag "{something}"
                ClientName souin-redis
                InitAddress 127.0.0.1:6379
                SelectDB 0
            }
        }
    }
    respond "Hello redis"
}

By the way I fixed the purge but the API is the following to flush all keys:

curl -X PURGE '{YOUR_DOMAIN}/{SOUIN_API_PATH}/flush'

You can try that using the latest commit a9f87ce02be5b871101db189345eb3c81ce01676:

xcaddy build  --with github.com/darkweak/souin/plugins/caddy@a9f87ce02be5b871101db189345eb3c81ce01676  --with github.com/darkweak/souin@a9f87ce02be5b871101db189345eb3c81ce01676

@HobMartin
Copy link
Contributor

Hey, @darkweak, thanks for adding hashtags. The keys are finally deleted, but I have a problem with the updated key name in this commit. As I see, you use a hashed key, but I need to have the host in the key name.

Let me explain my situation. I have a dynamic host handler in Caddy, and I've added a cache for those hosts. The problem arises when I need to purge the cache for a single hostname. I've been using regex in the API call to Souin, but with the hashed key, this method is no longer effective.

BTW, from Souin, get endpoint got a response without new hashed keys

@HobMartin
Copy link
Contributor

@darkweak what about of support caddy placeholders in hashtag name? This will be so helpful in my case.

@darkweak
Copy link
Owner

@HobMartin placeholders support is currently in progress here #514 🙂

@HobMartin
Copy link
Contributor

HobMartin commented Jun 17, 2024

@darkweak thanks for adding placeholders. There is awesome feature!

But I'm still having a problem with deleting keys by regex. Flush request works well, but regex didn't 😓

Also, I was seen in source code, and see that bulk delete using delete method from storage, how this method will delete keys by regex, maybe it should be DeleteMany that use scan command from Redis?

FYI: When I'm trying to delete key by ID that's works, but doesn't work with regex :((

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants