8000 Cache pool keys colide when Chain Adapter pool is used as a parent pool · Issue #49607 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Cache pool keys colide when Chain Adapter pool is used as a parent pool #49607

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.

Sign up for GitHub

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

Jump to bottom
Closed
Housik opened this issue Mar 5, 2023 · 7 comments
Closed

Cache pool keys colide when Chain Adapter pool is used as a parent pool #49607

Housik opened this issue Mar 5, 2023 · 7 comments
Labels
Bug Stalled Status: Needs Review

Comments

@Housik
Copy link
Housik commented Mar 5, 2023 •
edited
Loading

Symfony version(s) affected

6.2.5

Description

Current behaviour

Let's create following cache pool config:

framework:
    cache:
        prefix_seed: my-amazing-project

        app: cache.app.custom
        system: cache.adapter.system

        pools:
            cache.app.custom:
                adapters: cache.adapter.apcu
                clearer: cache.app_clearer
                tags: true
            cache.app.vokativ:
                adapter: cache.app
            cache.app.user:
                adapter: cache.app

In this case every pool referencing cache.app has its own namespace for generating keys to APCU, as declared in documentation, see bellow.

If I save item with key ipsum to cache cache.app.vokativ with value A and another item with the same key ipsum and value B to cache cache.app.user, cache records do not colide and from cache cache.app.vokativ is returned A value, from cache cache.app.user is returned B value, even if pools share the same backend cache.app (= cache.app.custom).

Now, let's have following buggy config:

We will modify main cache.app.custom to use Chain Adapter with APCU and Redis.

framework:
    cache:
        prefix_seed: my-amazing-project

        app: cache.app.custom
        system: cache.adapter.system

        default_redis_provider: '%env(resolve:REDIS_DSN)%'

        pools:
            cache.app.custom:
                adapters:
                    - cache.adapter.apcu
                    - cache.adapter.redis_tag_aware
                clearer: cache.app_clearer
                tags: true
            cache.app.vokativ:
                adapter: cache.app
            cache.app.user:
                adapter: cache.app

In this case every pool referencing cache.app has THE SAME NAMESPACE for generating keys saved to the cache.app pool.

If I save item with key ipsum with value A to cache cache.app.vokativ and another item with the same key ipsum and value B to cache cache.app.user, cache records do colide and from cache cache.app.vokativ is returned A value, from cache cache.app.user is returned A value, when the pools share the same backend cache.app (= cache.app.custom).

I have checked it manually in Redis and the namespace for keys is the same. It is happening only, if parent pool is the chain type pool.

The problem is happing here -> https://github.com/symfony/cache/blob/6.2/DependencyInjection/CachePoolPass.php#L58 between lines 58 and 65, which results that $tags[0]['name'] contains Service ID of Chained pool... so $id of pool is never used in this case. I do not know the reason, why the code designed this way... but anyway it is not corresponding to actual documentation.

From my point of view, even if I use chain adapter as parent adapter for new pool, new namespace should be introduced, otherwise cache key pools will colide and it will lead to unexpected cache pool results.

Expected behaviour

Symfony Docs -> Cache -> Creating Custom (Namespaced) Pools:
Each pool manages a set of independent cache keys: keys from different pools never collide, even if they share the same backend. This is achieved by prefixing keys with a namespace that's generated by hashing the name of the pool, the name of the cache adapter class and a configurable seed that defaults to the project directory and compiled container class.

How to reproduce

Use following config to reproduce problem. You can use any kind of cache adapters contained in the chain.

framework:
    cache:
        prefix_seed: my-amazing-project

        app: cache.app.custom
        system: cache.adapter.system

        default_redis_provider: '%env(resolve:REDIS_DSN)%'

        pools:
            cache.app.custom:
                adapters:
                    - cache.adapter.apcu
                    - cache.adapter.redis_tag_aware
                clearer: cache.app_clearer
                tags: true
            cache.app.vokativ:
                adapter: cache.app
            cache.app.user:
                adapter: cache.app

Possible Solution

Change the behavior in the loop beginning here https://github.com/symfony/cache/blob/6.2/DependencyInjection/CachePoolPass.php#L58
@Housik Housik added the Bug label Mar 5, 2023
@Housik Housik changed the title Cache pool keys colide when Chain Adapter pool is used as parent pool Cache pool keys colide when Chain Adapter pool is used as a parent pool Mar 5, 2023
@nicolas-grekas
Copy link
Member

Thanks for the detailed report. Would you like to send a PR to fix the issue?

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

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

@Housik
Copy link
Author
Housik commented Sep 28, 2023

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

Issue still needs to be investigated. I will try to do a pull request.

@carsonbot carsonbot removed the Stalled label Sep 28, 2023
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@nicolas-grekas
Copy link
Member

I think this is the same as #54097, which provides a nice discussion on the topic. TL;DR don't make cache.app point to a tag-aware pool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Stalled Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @Housik @carsonbot
0