Skip to content

Redis cache improvements #14724

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
Merged
merged 5 commits into from
Mar 17, 2025
Merged

Redis cache improvements #14724

merged 5 commits into from
Mar 17, 2025

Conversation

fedys
Copy link
Member

@fedys fedys commented Mar 12, 2025 •
edited
Loading

Q A
Bug fix? (use the a.b branch) ❌
New feature/enhancement? (use the a.x branch) ✔️
Deprecations? ❌
BC breaks? (use the c.x branch) ❌
Automated tests included? ✔️
Related user documentation PR URL
Related developer documentation PR URL
Issue(s) addressed

Description

This PR improves utilization of Redis backend when Mautic's cache configuration is set to Redis. Before this PR, Mautic was using a CacheProviderTagAwareInterface in situations when the tagable cache provider was not necessary and it was generating around 50% more queries to Redis. This PR separates the tag-aware cache provider from the non-tag-aware cache provider by introducing a new CacheProviderTagAwareInterface.

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Navigate to config/local.php and add the following lines to start using Redis as the cache backend:
    'cache_adapter'            => 'mautic.cache.adapter.redis',
    'cache_adapter_tag_aware'  => 'mautic.cache.adapter.redis_tag_aware',
    'cache_prefix'             => 'mautic',
    'cache_adapter_redis'      => [
        'dsn'     => 'redis://redis',
    ],
  1. Clear the cache by deleting the app/cache directory to make sure the above changes in config/local.php are applied.
  2. In the web browser navigate to the following sections and check if the basic functionality is working:
    • Dashboard: widget creation, updating and deletion.
    • Components -> Forms: form creation, updating and deletion.
    • Channels -> Focus Items: focus item creation, updating and deletion.

@codecov Codecov
Copy link

codecov bot commented Mar 12, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 61.53846% with 20 lines in your changes missing coverage. Please review.

Project coverage is 64.75%. Comparing base (80b22f2) to head (dad04b8).
Report is 32 commits behind head on 7.x.

Files with missing lines Patch % Lines
...undles/CacheBundle/Cache/AbstractCacheProvider.php 64.28% 10 Missing ⚠️
...es/CacheBundle/Cache/Adapter/RedisAdapterTrait.php 0.00% 6 Missing ⚠️
...bundles/CacheBundle/Cache/Adapter/RedisAdapter.php 0.00% 2 Missing ⚠️
...CacheBundle/Cache/Adapter/RedisTagAwareAdapter.php 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #14724      +/-   ##
============================================
+ Coverage     64.70%   64.75%   +0.04%     
- Complexity    34736    34826      +90     
============================================
  Files          2279     2288       +9     
  Lines        103738   103943     +205     
============================================
+ Hits          67123    67304     +181     
- Misses        36615    36639      +24
Files with missing lines Coverage Δ
app/bundles/CacheBundle/Cache/CacheProvider.php 100.00% <100.00%> (+30.30%) ⬆️
...undles/CacheBundle/Cache/CacheProviderTagAware.php 100.00% <100.00%> (ø)
...undle/DependencyInjection/MauticCacheExtension.php 100.00% <100.00%> (ø)
...undles/DashboardBundle/Event/WidgetDetailEvent.php 70.58% <100.00%> (ø)
...shboardBundle/Factory/WidgetDetailEventFactory.php 100.00% <ø> (ø)
...p/bundles/DashboardBundle/Model/DashboardModel.php 70.78% <ø> (ø)
...rmBundle/Collector/AlreadyMappedFieldCollector.php 100.00% <ø> (ø)
...ns/MauticFocusBundle/Controller/AjaxController.php 54.38% <100.00%> (ø)
...s/MauticFocusBundle/Controller/FocusController.php 64.55% <ø> (ø)
...bundles/CacheBundle/Cache/Adapter/RedisAdapter.php 0.00% <0.00%> (ø)
... and 3 more

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fedys fedys requested review from a team, escopecz and nileshlohar and removed request for a team March 13, 2025 11:35
@fedys fedys requested a review from Copilot March 13, 2025 11:37
Copilot
Copilot AI reviewed Mar 13, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.

@escopecz escopecz added ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality unforking Used for PRs in the Acquia's unforking initiative labels Mar 13, 2025
@escopecz escopecz moved this to 🦸🏻 Needs 2 tests in Open Source Fridays Mar 13, 2025
@escopecz escopecz added this to the 7.0.0-alpha milestone Mar 13, 2025
nileshlohar
nileshlohar reviewed Mar 13, 2025
View reviewed changes
app/bundles/CacheBundle/Config/services.php Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding this now ?
Also don't see this in original PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed for Symfony's auto-wiring which is not used in Symfony 4.

@escopecz escopecz requested a review from nileshlohar March 13, 2025 14:40
@fedys fedys requested a review from Copilot March 13, 2025 14:56
Copilot
Copilot AI reviewed Mar 13, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.

escopecz
escopecz approved these changes Mar 13, 2025
View reviewed changes
Copy link
Member

@escopecz escopecz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The code looks great and the going through the testing steps went smooth except the unrelated UI issue

Thank you!

@escopecz escopecz moved this from 🦸🏻 Needs 2 tests to ⏳︎ Needs 1 more test in Open Source Fridays Mar 13, 2025
@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged code-review-passed PRs which have passed code review and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Mar 13, 2025
@escopecz escopecz added the user-testing-passed PRs which have been successfully tested by the required number of people. label Mar 13, 2025
nileshlohar
nileshlohar approved these changes Mar 17, 2025
View reviewed changes
Copy link
Contributor

@nileshlohar nileshlohar left a comment

Choose a reason for hiding this comment

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

Code changes looks good.

@escopecz escopecz merged commit e524d07 into mautic:7.x Mar 17, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from ⏳︎ Needs 1 more test to 🥳 Done in Open Source Fridays Mar 17, 2025
@RCheesley
Copy link
Member

RCheesley commented Jul 24, 2025 •
edited
Loading

@fedys I'm not seeing any documentation PRs for this. It should at least be mentioned here, I think? Otherwise how do people know that it can be used?

https://docs.mautic.org/en/7.0/configuration/settings.html#redis

cc @mautic/education-team-leaders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@escopecz escopecz escopecz approved these changes

+1 more reviewer

@nileshlohar nileshlohar nileshlohar approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@escopecz escopecz

Labels
code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality pending-test-confirmation PR's that require one test before they can be merged unforking Used for PRs in the Acquia's unforking initiative user-testing-passed PRs which have been successfully tested by the required number of people.
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
7.0.0-alpha
Development

Successfully merging this pull request may close these issues.
4 participants
@fedys @RCheesley @escopecz @nileshlohar