Skip to content

DPMMA-3118 Segment build time optimization (Segment membership filter) #15031

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
Jul 22, 2025
Merged

DPMMA-3118 Segment build time optimization (Segment membership filter) #15031

merged 5 commits into from
Jul 22, 2025

Conversation

patrykgruszka
Copy link
Member

@patrykgruszka patrykgruszka commented May 22, 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 mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

This PR improves segment build performance by verifying contact membership in dependent segments, eliminating redundant query execution and reducing database load.

Changes included in this PR:

  • Optimized segment build queries by checking contact existence in dependent segments instead of duplicating queries.
  • Modified the segment update command to detect and rebuild dependent segments before rebuilding the main segment.
  • Added logic to handle circular dependencies and missing segments safely during rebuild.
  • [BC] Removed ContactSegmentFilterDictionary.php as unused.

Optimization results observed in the test environment:
Before this PR: Total time was 673.18 seconds
After this PR: Total time reduced to 80.16 seconds

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Prepare segments with dependencies (e.g., Segment A includes Segment B).
  3. Run the mautic:segments:update command for a segment with dependencies.
  4. Verify that dependent segments are rebuilt before the main segment.
  5. Confirm that output messages are clear and reflect the rebuild order and any issues detected.

@patrykgruszka patrykgruszka marked this pull request as draft May 22, 2025 13:52
@patrykgruszka patrykgruszka force-pushed the DPMMA-3118_segment-static-m7 branch from e9f9111 to f62d842 Compare May 22, 2025 14:03
@patrykgruszka
fix: [DPMMA-3118] segment static filters
1e22020 
fix: [DPMMA-3118] breaking change info and cleanup
@patrykgruszka patrykgruszka force-pushed the DPMMA-3118_segment-static-m7 branch from f62d842 to 1e22020 Compare May 22, 2025 14:07
@codecov Codecov
Copy link

codecov bot commented May 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 98.59155% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.64%. Comparing base (6803ff6) to head (29bc39a).
⚠️ Report is 421 commits behind head on 7.x.

Files with missing lines Patch % Lines
...dles/LeadBundle/Command/UpdateLeadListsCommand.php 98.50% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                7.x   #15031   +/-   ##
=========================================
  Coverage     66.64%   66.64%           
  Complexity    35046    35046           
=========================================
  Files          2300     2299    -1     
  Lines        141364   141371    +7     
=========================================
+ Hits          94210    94220   +10     
+ Misses        47154    47151    -3
Files with missing lines Coverage Δ
...Bundle/Services/ContactSegmentFilterDictionary.php 100.00% <100.00%> (ø)
...dles/LeadBundle/Command/UpdateLeadListsCommand.php 98.58% <98.50%> (-0.41%) ⬇️

... and 2 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.

@patrykgruszka patrykgruszka marked this pull request as ready for review May 23, 2025 09:49
@patrykgruszka patrykgruszka added ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging segments Anything related to segments performance-scalability Anything related to performance and scalability labels May 23, 2025
@patrykgruszka patrykgruszka mentioned this pull request May 23, 2025
Closed
@patrykgruszka patrykgruszka requested review from escopecz and kuzmany May 23, 2025 09:54
@patrykgruszka patrykgruszka changed the title DPMMA-3118 Segment build time optimization (static membership filter) DPMMA-3118 Segment build time optimization (Segment membership filter) May 23, 2025
@patrykgruszka patrykgruszka moved this to 🧑🏻‍💻 Needs a code review in Open Source Fridays May 23, 2025
kuzmany
kuzmany approved these changes May 27, 2025
View reviewed changes
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Seems good to me 👍
Code looks legit
I tested few filters, query seems correct generated.
Well done

@kuzmany kuzmany 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 code-review-needed PR's that require a code review before merging ready-to-test PR's that are ready to test labels May 27, 2025
@patrykgruszka patrykgruszka moved this from 🧑🏻‍💻 Needs a code review to ⏳︎ Needs 1 more test in Open Source Fridays May 29, 2025
@patrykgruszka
Copy link
Member Author

After testing phase we've deployed this on production and this solution has reduced segment rebuild time by 99%. No issues have been observed so far.

@RCheesley
Copy link
Member

Just a small conflict to fix here @patrykgruszka then we should be good to go!

@RCheesley RCheesley added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Jul 21, 2025
@RCheesley RCheesley moved this to 🎉 Ready to commit in Open Source Fridays Jul 21, 2025
@RCheesley RCheesley added this to the 7.0.0-alpha milestone Jul 21, 2025
@patrykgruszka
Copy link
Member Author

@RCheesley, thanks for the heads-up, done :)

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.4% Duplication on New Code

See analysis details on SonarQube Cloud

@RCheesley RCheesley merged commit 6ebfbba into mautic:7.x Jul 22, 2025
24 checks passed
@github-project-automation github-project-automation bot moved this from 🎉 Ready to commit to 🥳 Done in Open Source Fridays Jul 22, 2025
andersonjeccel pushed a commit to andersonjeccel/mautic that referenced this pull request Jul 23, 2025
@patrykgruszka @RCheesley @andersonjeccel
DPMMA-3118 Segment build time optimization (Segment membership filter) (
51aa59f 
mautic#15031)

* fix: [DPMMA-3118] segment static filters

fix: [DPMMA-3118] breaking change info and cleanup

* fix: [DPMMA-3118] phpstan and csfixer

* fix: [DPMMA-3118] segment functional tests

---------

Co-authored-by: Ruth Cheesley <ruth@ruthcheesley.co.uk>
@kuzmany kuzmany mentioned this pull request Jul 31, 2025
Closed
@escopecz escopecz added the enhancement Any improvement to an existing feature or functionality label Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kuzmany kuzmany kuzmany approved these changes

@escopecz escopecz Awaiting requested review from escopecz

Assignees
No one assigned
Labels
code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality performance-scalability Anything related to performance and scalability ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged segments Anything related to segments
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
7.0.0-alpha
Development

Successfully merging this pull request may close these issues.
4 participants
@patrykgruszka @RCheesley @kuzmany @escopecz