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

FEATURE: Pass tags to be flushed to content cache backend #3631

Merged
merged 5 commits into from Mar 23, 2022

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Mar 2, 2022

What I did

Instead of calling the cache backend for each tag to flush
individually, the list of tags is passed to the backend with
the newly introduced flushByTags method in
neos/flow-development-collection#2718.

This allows each type of backend to optimise the flushing
of all tags, which can lead to huge performance improvements.
Especially when content is published to the live workspace
which leads to large numbers of cache tags that will be flushed.

Also the messages stored with individual content cache tags
take up a lot of unnecessary memory in production
context and not are even used there.

With this change the behaviour can be enabled via
the setting Neos.Neos.fusion.contentCacheDebugMode.

Resolves: #3640

How I did it

The ContentCacheFlusher now calls the flushByTags method introduced in neos/flow-development-collection#2718.

Only in the newly introduced debug mode the old style of flushing by tag individually is used to provide the individual logged feedback why entries were flushed.

How to verify it

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch

Instead of calling the cache backend for each tag to flush
individually, the list of tags is passed to the backend with
the newly introduced `flushByTags` method in
neos/flow-development-collection#2718.

This allows each type of backend to optimise the flushing
of all tags, which can lead to huge performance improvements.
Especially when content is published to the live workspace
which leads to large numbers of cache tags that will be flushed.
@Sebobo Sebobo added the Feature label Mar 2, 2022
The messages stored with individual content cache tags
take up a lot of unnecessary memory in production
context and not are even used there.

With this change the behaviour can be enabled via
the setting `Neos.Neos.fusion.contentCacheDebugMode`.
@Sebobo Sebobo self-assigned this Mar 2, 2022
@Sebobo Sebobo changed the title FEATURE: Pass tags to be flushed to cache backend FEATURE: Pass tags to be flushed to content cache backend Mar 3, 2022
This helps when trying to use AOP to influence caching behaviour.
The RouteCacheFlusher does it the same way.
This can speed up flushes after publishing a lot if a lot of document nodes
are affected by the published changes.
Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works and makes sense.

One thing that could be added would be to enable the debugMode in development context by default.

The tests can only turn green with neos/flow-development-collection#2718 merged into flow.

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about enabling debug mode in development context by default?

Neos.Neos/Classes/Fusion/Cache/ContentCacheFlusher.php Outdated Show resolved Hide resolved
Co-authored-by: Karsten Dambekalns <karsten@dambekalns.de>
@Sebobo
Copy link
Member Author

Sebobo commented Mar 18, 2022

I think inspecting the atomic cache debug output is a very specific use case, IMO this should be left disabled until needed.
The output whether entries were flushed at all and how many tags were flushed is still there independent of the new debug flag.
We spend so much time in Development context and there is no need to make it slower than it must be.

@mficzel
Copy link
Member

mficzel commented Mar 20, 2022

I agree to the argument of @Sebobo that not every flushed tag should be logged even in dev-mode. It can be enabled for debugging caches but other than that i is a risk of covering up important log messages.

@kdambekalns kdambekalns merged commit 52cdc91 into neos:master Mar 23, 2022
@Sebobo Sebobo deleted the feature/flush-tags branch March 23, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

FEATURE: Flush multiple tags at once in FusionContentCache
3 participants