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

Reduce deadlocks on inserting custom data by only using 'ON DUPLICATE' when it is not a new row #14605

Merged
merged 1 commit into from Jun 23, 2019

Conversation

@eileenmcnaughton
Copy link
Contributor

commented Jun 22, 2019

Overview

Reduce deadlocks when processing contributions with custom data. This specifically makes the custom data insert sql more efficient. The approach could be extended to other entities. We deployed this in production before our main fundraiser in Dec last year and it noticeably decreased deadlocks

Before

More frequent deadlocks

After

Less frequent deadlocks

Technical Details

ON Duplicate is used in the customData.create function so that a row can be inserted and if it exists
mysql adapts and updates the existing row. This is more expensive and more prone to deadlocks than a straight
'INSERT' but is probably better than figuring it out at the php layer when you don't know if it could be
an update rather than an insert.

However, in many of the cases we already know this information - ie. if we are creating a new contribution
the custom data is created afterwards so we can use this information from Contribution.create to opt for the
cheaper & less deadlocky version.

We deployed this fix before our main fundraiser due to handful of daily deadlocks on this under peak load
and this form of deadlock did not bother us again during our main fundraiser when we were processing large volumes.
The patch has been in production around 6 months at this point.

Note that the reason deadlocks are encountered is that the 'next row' index is locked when inserting, and
it's either locked for longer or more aggressively when it;'s also checking for a deadlock at the same time (not
sure which)

Comments

@pfigel might affect you?

Reduce deadlocks on inserting custom data by only using 'ON DUPLICATE…
…' when it is not a new row

ON Duplicate is used in the customData.create function so that a row can be inserted and if it exists
mysql adapts and updates the existing row. This is more expensive and more prone to deadlocks than a straight
'INSERT' but is probably better than figuring it out at the php layer when you don't know if it could be
an update rather than an insert.

However, in many of the cases we already know this information - ie. if we are creating a new contribution
the custom data is created afterwards so we can use this information from Contribution.create to opt for the
cheaper & less deadlocky version.

We deployed this fix before our main fundraiser due to handful of daily deadlocks on this under peak load
and this form of deadlock did not bother us again during our main fundraiser when we were processing large volumes.
The patch has been in production around 6 months at this point.

Note that the reason deadlocks are encountered is that the 'next row' index is locked when inserting, and
it's either locked for longer or more aggressively when it;'s also checking for a deadlock at the same time (not
sure which)
@civibot

This comment has been minimized.

Copy link

commented Jun 22, 2019

(Standard links)

@civibot civibot bot added the master label Jun 22, 2019

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2019

Jenkins re test this please

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

This seems pretty straightforward to me and we have plenty of tests covering custom data at least via the API so feel relatively confident with this if Jenkins is ok with it

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

Jenkins re test this please

@eileenmcnaughton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2019

thanks @seamuslee001 this has also been production-thrashed :-)

@seamuslee001

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

Adding merge on pass

@eileenmcnaughton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2019

test fails are unrelated

@eileenmcnaughton eileenmcnaughton merged commit 287d622 into civicrm:master Jun 23, 2019

1 check failed

default Build finished.
Details

@eileenmcnaughton eileenmcnaughton deleted the eileenmcnaughton:cust_deadlock branch Jun 23, 2019

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.