-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Allow creating chained cache pools by providing several adapters #32294
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
Conversation
c058427
to
b57f063
Compare
For reference, this leverages a new convention supported by the my_chain_cache:
class: Symony\Component\Cache\Adapter\ChainAdapter
arguments:
0: cache.adapter.array
redis://foo: cache.adapter.redis This is a bit strange I agree, yet this should be pretty rare - it's mostly for bundles that couldn't use the fwb configuration. |
For reference too, it should be possible to add a local memory cache to all "app" pools: framework:
cache:
app: cache.app.chain
pools:
api_cache: ~ # "api_cache" pool uses "cache.app" as template, which is a chain
cache.app.chain:
adapters:
- cache.adapter.array
- cache.adapter.filesystem # or any other backend (this is combining knowledge that the doc already gives - or will after symfony/symfony-docs#11855) /cc @andrerom you might like this too :) |
I like it But if you plan to expose use of ArrayAdapter to a wider audience you should probably also add the following features to it:
If you want to be advance you can also:
We opted to not do this as a PSR Pool in eZ Platform 2.5 precisely for the last reason here, some data domains has a in-memory TTL of a few seconds, others just some hundred milliseconds. |
b57f063
to
dc0db63
Compare
Thank you for the ideas @andrerom |
Normally array pool will be empty on each new request or command execution right? Or are you talking about ReactPHP or future 7.4's pre-loading scenarios? For normal fpm setup the start of the request is not an issue, it's for instance when something is long lived it becomes increasingly an issue, request or command. That said, as long as doc does not use array as an example you can move ahead with this and we can try to make it more safe to use before 4.4 is out in order to document usage of it (including full pros/cons). |
yes, that's what I'm referring to. I've actually submitted #32361 to reset array pools, this was forgotten. But I agree, let's deal with these concerns separately. PR is ready. |
dc0db63
to
24108bc
Compare
…nsionConfigurationPass (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] fix processing of regular parameter bags by MergeExtensionConfigurationPass | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Spotted in and needed by #32294 Commits ------- b06d000 [DI] fix processing of regular parameter bags by MergeExtensionConfigurationPass
24108bc
to
d92d864
Compare
d92d864
to
29ba091
Compare
Thank you @nicolas-grekas. |
…y providing several adapters (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] Allow creating chained cache pools by providing several adapters | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Replaces #30984, follows symfony/symfony-docs#11813 This PR allows defining several adapters for one pool. When doing so, this defines a chain pool. The benefit is that all chained pools get automatic namespace and lifetime, so things are transparent: ```yaml pools: my_chained_pool: default_lifetime: 12 adapters: - cache.adapter.array - cache.adapter.filesystem - {name: cache.adapter.redis, provider: 'redis://foo'} ``` (see fixtures for example of PHP/XML config) /cc @Nyholm @pborreli FYI Commits ------- 29ba091 [FrameworkBundle] Allow creating chained cache pools by providing several adapters
…(Nyholm) This PR was submitted for the master branch but it was squashed and merged into the 4.4 branch instead (closes #11855). Discussion ---------- Document how to create cache pool with framework config This is some docs for symfony/symfony#32294 I removed the "old" way of doing it because this is far better. Commits ------- b0dc28f Document how to create cache pool with framework config
…che (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [Cache] Add LRU + max-lifetime capabilities to ArrayCache | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix https://github.com/orgs/symfony/projects/1#card-30686676 | License | MIT | Doc PR | - In #32294 (comment), @andrerom writes: > if you plan to expose use of ArrayAdapter to a wider audience you should probably also add the following features to it: > - max item limit to avoid reaching memory limits > - own (very low, like default 100-500ms) TTL for in-memory caching, as it's in practice stale data when used in concurrent scenarios > > If you want to be advance you can also: > > - keep track of use, and evict cache items based on that using LFU when reaching limit > - in-memory cache is domain & project specific in terms of how long it's somewhat "safe" to keep items in memory, so either describe when to use and not use on a per pool term, or allow use of pool to pass in flags to opt out of in-memory cache for cases developer knows it should be ignored This PR implements these suggestions, via two new constructor arguments: `$maxLifetime` and `$maxItems`. In Yaml: ```yaml services: app.lru150_cache: parent: cache.adapter.array arguments: $maxItems: 150 $maxLifetime: 0.150 framework: cache: pools: my_chained_pool: adapters: - app.lru150_cache - cache.adapter.filesystem ``` This configuration adds a local memory cache that keeps max 150 elements for 150ms on top of a filesystem cache. /cc @lyrixx since you were also interested in it. Commits ------- 48a5d5e [Cache] Add LRU + max-lifetime capabilities to ArrayCache
…che (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [Cache] Add LRU + max-lifetime capabilities to ArrayCache | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix https://github.com/orgs/symfony/projects/1#card-30686676 | License | MIT | Doc PR | - In symfony/symfony#32294 (comment), @andrerom writes: > if you plan to expose use of ArrayAdapter to a wider audience you should probably also add the following features to it: > - max item limit to avoid reaching memory limits > - own (very low, like default 100-500ms) TTL for in-memory caching, as it's in practice stale data when used in concurrent scenarios > > If you want to be advance you can also: > > - keep track of use, and evict cache items based on that using LFU when reaching limit > - in-memory cache is domain & project specific in terms of how long it's somewhat "safe" to keep items in memory, so either describe when to use and not use on a per pool term, or allow use of pool to pass in flags to opt out of in-memory cache for cases developer knows it should be ignored This PR implements these suggestions, via two new constructor arguments: `$maxLifetime` and `$maxItems`. In Yaml: ```yaml services: app.lru150_cache: parent: cache.adapter.array arguments: $maxItems: 150 $maxLifetime: 0.150 framework: cache: pools: my_chained_pool: adapters: - app.lru150_cache - cache.adapter.filesystem ``` This configuration adds a local memory cache that keeps max 150 elements for 150ms on top of a filesystem cache. /cc @lyrixx since you were also interested in it. Commits ------- 48a5d5e8a9 [Cache] Add LRU + max-lifetime capabilities to ArrayCache
Replaces #30984, follows symfony/symfony-docs#11813
This PR allows defining several adapters for one pool. When doing so, this defines a chain pool.
The benefit is that all chained pools get automatic namespace and lifetime, so things are transparent:
(see fixtures for example of PHP/XML config)
/cc @Nyholm @pborreli FYI