-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache] Allow colon in key key (with redis at least) #45599
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
Comments
I'd be fine extending the range of allowed characters when the Symfony contracts are used. |
Thank you for this issue. |
I fail to see what we could do here if PSR-6/PSR-16 are not changed. |
We can skip this recommendation? 🤷🏼 |
BTW, We already broke this recommendation. We use a colon between the namespace (prefix seed), and the real key. so I'm gonna do a PR to remove this useless limitation. |
We did not break anything, the colon we use is not on a PSR-6 boundary so the spec doesn't apply there. It's actually a good thing since it means public keys cannot mess up with internal (prefixed) keys. |
What about my proposal above? We can change our contracts. |
yes, that's what I did 👍 |
It is not a good idea. It will break some implementations at least. Cache adapters can use any characters that are supported by their backends. If some backend does not support some character, the adapter has to deal with it in some way. So it is practical when the caching interface supports only a subset of common characters. Some adapters use reserved characters to replace unsupported characters on the backend (e.g. MemcachedAdapter), others use escaping that increases a key size. Namespaces are needed for preventing collisions of names. It is convenient to have a few adapters with different namespaces instead of one with long prefixed keys. The This example: $app = new RedisAdapter($client, 'application:module:component_one');
$sys = new RedisAdapter($client, 'system');
$app->hasItem('the_key'); // in Redis it would be "application:module:component_one:the_key"
$sys->hasItem('the_key'); // in Redis it would be "system:the_key" is much better than next one: $cache = new RedisAdapter($client);
$cache->hasItem('application:module:component_one:the_key');
$cache->hasItem('system:the_key'); In the second example, it is always needed to prefix keys. Currently, namespaces can't contain a colon (it does |
@upyx IIUC, it means I will have to create many pools with different namespaces. In one class of my project, I think I use at least 20 different prefix! More over, I also use some ID in a namespace, and so it's not possible to create every possible combinaison in the static (config.yml) configuration. Some code to illustrate what I'm talking about: public function startCrawl(string $crawlId, array $config)
{
$this->redis()->hMSet("crawler:{$crawlId}:configuration", $config);
// Some Gauges, values keep incrementing:
$this->redis()->tsCreate("crawler:{$crawlId}:ts:g:crawlable");
$this->redis()->tsAdd("crawler:{$crawlId}:ts:g:crawlable", null, $this->redis()->sCard("crawler:{$crawlId}:crawlable"));
$this->redis()->tsCreate("crawler:{$crawlId}:ts:g:downloaded");
$this->redis()->tsAdd("crawler:{$crawlId}:ts:g:downloaded", null, 0);
$this->redis()->tsCreate("crawler:{$crawlId}:ts:g:analyzed");
$this->redis()->tsAdd("crawler:{$crawlId}:ts:g:analyzed", null, 0);
$this->redis()->tsCreate("crawler:{$crawlId}:ts:g:crawled");
$this->redis()->tsAdd("crawler:{$crawlId}:ts:g:crawled", null, 0);
[...] |
Looks a bit creepy, do you know? 😂 So, you have nested namespaces. Let's look at this in different way: public function startCrawl(string $crawlId, array $config)
{
$redis = $this->redis("crawler:{$crawlId}");
$redisTs = $redis->nest("ts:g");
$redis->hMSet("configuration", $config);
$redisTs->tsCreate("crawlable");
$redisTs->tsAdd("crawlable", null, $redis->sCard("crawlable"));
$redisTs->tsCreate("downloaded");
$redisTs->tsAdd("downloaded", null, 0);
$redisTs->tsCreate("analyzed");
$redisTs->tsAdd("analyzed", null, 0);
$redisTs->tsCreate("crawled");
$redisTs->tsAdd("crawled", null, 0);
// other
} A bit clearer, isn't it? The Symfony Cache does not help here. And changing the interface will not help too.
Yes. A factory that will create pools on the fly is needed. I imagine it like framework:
cache:
crawler: '%env(REDIS_DSN)%' class Crawler {
private CacheInterface $cache;
private array $config;
public function __construct(CacheInterface $cacheCrawler, string $crawlId, array $config)
{
$this->cache = $cacheCrawler->nest($crawlId);
$this->config = $config;
}
public function start()
{
$tsg = $this->cache->nest("ts:g");
$etc = $this->cache->nest("some:other:etc");
// ...
}
} Something like this... It's just an idea. |
I like Note that ProxyAdapter can be used for this use case: But having a dedicated method + interface could be nice. |
Even if
It is basic string concatenation... I use this everywhere! I understand some people might find this ugly, but this is not. So IMHO this should be OK to compose manually the key and avoid manipulating too much object to achieve a very simple goal! |
Oh, nothing wrong with concatenation! I talked about repeating and breaking the DRY principle which is not important to the example. I'm very sorry that it touched you. |
Thank you for this issue. |
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
Keep it open |
FTR, the PR on php-fig/fig-standards has been refused |
Colons are dedicated to namespace separator. So you can add namespaces to cache providers, and they will be saparated from keys by colons. A namespace can be set directly to There are three typical ways to do so:
|
Does not work, the key of the cache item is appended directly to the sub-namespace without using the ':' separator. |
Can you try adding the colon to the namespace? |
@nicolas-grekas PS: I'm in symfony 6.4.16 |
Oh, then this should maybe be relaxed, this constructor argument doesn't have to adhere to PSR-6 rules. |
Hi! When do you think we will have ":" available as an available character? |
Description
I noticed something strange with symfony cache. It forbids to use a colon
:
in the key name.But, namespacing keys with redis is (was?) a best practice, and so we lose some power when using redis insight for example.
In insight, I can profile redis, and it shows me what keys I use the most by grouping keys by pattern (
a:b:c:*
)As you can see, the "TOP KEY PATTERNS" is poorly detailed.
f-yOWMrlsA:*
on the screenshot)So to me, we should simply remove this restriction
Note: psr restricts this usage
But I don't use PSR, And I personally don't care about this PSR since I'm using Symfony contracts.
Example
No response
The text was updated successfully, but these errors were encountered: