We're getting deadlocks when the recurring queue consumer runs at the same time as the donations queue consumer.
Let's make the recurring queue consumer strictly responsible for starting/ending new subscriptions.
The normal donations queue consumer IS capable of handling some recurring payments, but we need to move a little bit of normalization from the recurring queue consumer to the main normalization function.
Change 556829 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[wikimedia/fundraising/SmashPig@master] WIP send recurring payments to donations queue
So there is indeed some recurring-specific normalization happening in the RecurringQueueConsumer, mostly stuff specific to paypal messages. We'd have to move that over to someplace in the donations queue consumer before we merge the above patch.
The recurring-specific normalization has two parts: 1) finding the contribution_tracking_id and 2) dealing with paypal inconsistencies in migrating legacy to new subscription ID formats.
The first part of this task is to redirect PayPal recurring payment instalments to the donations queue to be processed by the donations queue consumer. We have a WIP patch up for that.
The second part is to transfer the payment-import-specific code that currently exists in the recurring queue consumer over to the donations queue consumer. The code does two things: first, it adds required queue message properties to the payment message before importing it, and it then updates the recurring contribution record with the subscription's next scheduled date and status after the payment has been successfully imported.
The recurring queue consumer does other interesting things during a payment's import, such as reactivating previously cancelled or failed recurring subscriptions and, in some cases, creating entirely new ones. However, we can leave that out for now or add it as a third part of the work once we understand it better.
Change 969975 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] WIP: Send recurring payment instalments to the donations queue
Change 969976 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] WIP: Send recurring payment instalments to the donations queue
Change 970458 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Contribution Import Refactor: remove old code and group recurring behaviour
Change 970460 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Contribution Import Refactor: inline calls to ImportStatsCollector::getInstance()
Change 970462 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Contribution Import Refactor: Move main timer naming to fn
Change 970458 merged by jenkins-bot:
[wikimedia/fundraising/crm@master] Contribution Import Refactor: remove old code
Change 971206 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Contribution Import Refactor: extract out the timer start/end pepperings
Change 971212 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] WIP: Contribution Import Refactor: combine and simplify the recurring logic
Change 970460 merged by jenkins-bot:
[wikimedia/fundraising/crm@master] Contribution Import Refactor: inline calls to ImportStatsCollector::getInstance()
Change 970462 merged by jenkins-bot:
[wikimedia/fundraising/crm@master] Contribution Import Refactor: Move main timer naming to fn
Change 971206 merged by jenkins-bot:
[wikimedia/fundraising/crm@master] Contribution Import Refactor: extract out the timer start/end pepperings
Change 975286 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Move subscription handling to a separate class
Change 975287 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Move contribution import to a separate class
Change 975288 had a related patch set uploaded (by Jgleeson; author: Jgleeson):
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Return types and method reordering
Change 969976 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] WIP: Donation Consumer recurring payments support. Phase 2
Reason:
not needed
hey @Ejegg I'm trying to come up with a suitable approach to address how we currently handle recurring contributions with and without a related subscription across DonationQueueConsumer and RecurringQueueConsumer.
In DonationQueueConsumer, we permit recurring donations without a related subscription. When this scenario occurs, we create a new subscription and link the newly imported contribution to that subscription as part of the wmf_civicrm_contribution_message_import() call. There's a comment here explaining that this can happen for audit contributions.
In RecuringQueueConsumer, we reject recurring contributions which do not have a related subscription. Checks in the code here ensure a recurring contribution has a related subscription. If not, we throw a MISSING_PREDECESSOR exception with this message "recurring: Msg does not have a matching recurring record in civicrm_contribution_recur; requeueing for future processing."
As part of this work, I've moved the code that previously handled RecuringQueueConsumer::importSubscriptionPayment() to its own class here, and the DonationQueueConsumer, which will now process messages previously sent to RecuringQueueConsumer, will call the new RecurringSubscriptionPayment::importSubscriptionPayment() if the donation is marked as recurring, which is currently leading to the exception being thrown in cases we don't want it thrown. This is where the conflict arises.
To resolve this, I've started down this path, but I actually think that a better approach is to introduce subtypes of the RecurringSubscriptionPayment and handle the PayPal-specific behaviour that way. I'd like to hear your thoughts on that approach and also if you could explain briefly the history behind the difference in handling across both queue consumers. Thanks!
Change 971212 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] Contribution Import Refactor: combine recurring processing code
Reason:
Change 975286 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Move subscription handling to a separate class
Reason:
Change 975287 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Move contribution import to a separate class
Reason:
Change 969975 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] WIP: Donations Queue updates: Handle recurring instalments
Reason:
Change 975288 abandoned by Jgleeson:
[wikimedia/fundraising/crm@master] Recurring Queue restructure: Return types and method reordering
Reason:
We are closing in on this but need to resolve https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/crm/+/1020401
Change #1031093 had a related patch set uploaded (by Eileen; author: Eileen):
[wikimedia/fundraising/crm@master] Switch to a slightly less magic way of denoting how address should be updated
Change #1031108 had a related patch set uploaded (by Eileen; author: Eileen):
[wikimedia/fundraising/crm@master] Move the main message import to the DonationQueue handler
Change #1031107 had a related patch set uploaded (by Eileen; author: Eileen):
[wikimedia/fundraising/crm@master] Finally push the recurring donation to the donation queue
Once this patch https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/crm/+/1031626 is merged they will be pushed to the queue - although we could consider
Maybe those are outside the scope of this phab?