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

Duplicate internal IPs from campaign_lead_event_log with master/slave connections. #7806

Open
wants to merge 3 commits into
base: staging
from

Conversation

@heathdutton
Copy link
Member

heathdutton commented Aug 20, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks? N
Deprecations? N

Description:

When campaign events are triggered by cron tasks, we still spend a few CPU cycles looking up the IP and creating a reference to a local IP. This by itself isn't a problem, however, at high scale (read and write replicas and concurrent cron threads) the problem is multiplied because of the object relationship. The lookup can fail, and new IpAddress entities are created in huge quantities. Similar to #7552 a huge number of duplicate ip_address entities can be created, threatening the MAX_INT limitations. Since ip_address table is tied to 14 foreign key constraints, this could bring a production Mautic instance to a very bad place.

To save both CPU cycles, and to avoid accidental duplication we should avoid trying to log IpAddress entries for cron executed campaign actions.

Steps to reproduce the bug:

  1. Use a master/slave database.
  2. Run 40+ concurrent campaign execution threads.
  3. Note the ip_address table is flooded with local IP entities that shouldn't exist.

(Seriously though, there are maybe 4 companies using Mautic in this way that can replicate this)

Steps to test this PR:

  1. Load up this PR
  2. You should be able to create a campaign...
  3. Push one contact through it...
  4. Then view the contact, and edit the contact with no breakage.

This just confirms that there is not an accidental hard dependency on the ipAddress foreign key.

@heathdutton heathdutton changed the title Duplicate internal ips Duplicate internal IPs from campaign_lead_event_log with master/slave connections. Aug 20, 2019
@heathdutton

This comment has been minimized.

Copy link
Member Author

heathdutton commented Sep 13, 2019

We are running this in production now, and have been for a while. Seems solid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.