jamessimone / apex-rollup

Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.
MIT License
219 stars 30 forks source link

Concat Duplicating Values when Queueable and Synchronous Rollup Control Used on Same Parent/Child #596

Closed Silchuki14 closed 2 months ago

Silchuki14 commented 3 months ago

(First time using Github, hoping this is detailed enough)

Hello James, we are wondering if the fact we have a mix of Queueable and Synchronous Rollup Control on the same Child/Parent relationship is causing an issue with Contact where it would duplicate value. Below are our troubleshooting records that hopefully can help answer the question. Debug logs attached as well.

Please let me know if you have any questions, or would need additional details.

Thank you!


ApexRollup V1.6.23

Test Records:

1 Terminal_Upgradec Record (Parent) 2+ Device_to_Upgradec Records (Child(s))

Device to Upgrade Apex Trigger

trigger ApexRollup_DevicetoUpgradeTrigger on Device_to_Upgrade__c(after insert, after update, before delete, after undelete) {
  // after delete is required ONLY if your org does Account / Contact / Lead / Case merges
  Rollup.runFromTrigger();
  // etc. You can invoke the above from your handler if you have one
}

*_DLRS Apex Trigger is disabled_**

DLRS Comparables Concat_Distinct on DevUps_Models__c field d1621e24-157e-4a2a-ba0a-93b8bd17789f

Concat_Distinct on DevUps_Serial_Numbers__c field. 979fce0f-7c24-4bdc-853e-c47ad2250334

Rollup Controls and Rollup Configs on this “simple” Concat Rollup. cd7368cb-252e-41a0-8a60-c1995dd438ac *Queuable and not Synchronous because the former is understood as the preferred method.

*Rollup Logging enable for the purpose of this exercise. 2b93a53e-77c8-4526-b640-83f1fe35254f f6b0863b-1137-4f9a-81ee-925bccef3de3

Concat Issue

We noticed that the rollup sometimes acts like Concat_Distinct, or the value are duplicated in a way that there are more values than actual records. *For the purpose of this exercise, both Rollup have been updated to no delimiter (comma as default). Model is using Queueable Control and Serial Number is using Synchronous Control for comparison purposes.

Before doing any test, a Full Recalc on both Rollups has been performed. We start with the below field value: Models: DevUp098, DevUp123 Serial Numbers: 098765, 123456

Test #1 - Create a new Device to Upgrade from Scratch (not cloned) - Different Values - Parent Field Populated (Fail) Model: Queueable - Serial Number: Synchronous Model Added: DevUp111 - Serial Number Added: 111234 Expectation: Models: DevUp098, DevUp123, DevUp111 - Serial Numbers : 098765, 123456, 111234 Result: Models: DevUp098, DevUp098, DevUp111, DevUp123, DevUp123 - Serial Numbers: 098765, 111234, 123456

Concat - Test1CreateNewDeviceDiffValueNoClonePopulatedMixControl1.log Concat - Test1CreateNewDeviceDiffValueNoClonePopulatedMixControl2.log Concat - Test1CreateNewDeviceDiffValueNoClonePopulatedMixControl3.log

Test #2 - Delete Created Device to Upgrade (Fail) Model: Queueable - Serial Number: Synchronous Model Added: DevUp111 - Serial Number Added: 111234 Expectation: Models: DevUp098, DevUp123 - Serial Numbers : 098765, 123456 Result: Models: DevUp098, DevUp098, DevUp098, DevUp111, DevUp123, DevUp123, DevUp123 - Serial Numbers: 098765, 123456

Concat - Test2DeleteNewDeviceDiffValueNoClonePopulatedMixControl1.log Concat - Test2DeleteNewDeviceDiffValueNoClonePopulatedMixControl2.log Concat - Test2DeleteNewDeviceDiffValueNoClonePopulatedMixControl3.log

*Manually updated Parent Fields to what they were before Test #1. Updated Serial Number Rollup Control to Queueable in case the mix is causing the issue.

Test #3 - Create a new Device to Upgrade from Scratch (not cloned) - Different Values - Parent Field Populated (Pass) Model: Queueable - Serial Number: Queueable Model Added: DevUp111 - Serial Number Added: 111234 Expectation: Models: DevUp098, DevUp123, DevUp111 - Serial Numbers : 098765, 123456, 111234 Result: Models: DevUp098, DevUp111, DevUp123 - Serial Numbers: 098765, 111234, 123456

Concat - Test3CreateNewDeviceDiffValueNoClonePopulatedSameControl1.log Concat - Test3CreateNewDeviceDiffValueNoClonePopulatedSameControl2.log Concat - Test3CreateNewDeviceDiffValueNoClonePopulatedSameControl3.log

Test #4 - Create new Device to Upgrade by Cloning (DevUp111) - Same Values - Parent Field Populated (Pass) Model: Queueable - Serial Number: Queueable Model Added: DevUp111 - Serial Number Added: 111234 Expectation: Models: DevUp098, DevUp123, DevUp111, DevUp111 - Serial Numbers : 098765, 123456, 111234, 111234 Result: Models: DevUp098, DevUp111, DevUp111, DevUp123 - Serial Numbers: 098765, 111234, 111234, 123456

Concat - Test4CreateNewDeviceSameValueClonePopulatedSameControl1.log Concat - Test4CreateNewDeviceSameValueClonePopulatedSameControl2.log Concat - Test4CreateNewDeviceSameValueClonePopulatedSameControl3.log

Test#5 - Delete Cloned Device to Upgrade (DevUp111) (Pass) Model: Queueable - Serial Number: Queueable Model Deleted: DevUp111 - Serial Number Added: 111234 Expectation: Models: DevUp098, DevUp123, DevUp111 - Serial Numbers : 098765, 123456, 111234 Result: Models: DevUp098, DevUp111, DevUp123 - Serial Numbers: 098765, 111234, 123456

Concat - Test5DeleteNewDeviceDiffValueClonePopulatedSameControl1.log Concat - Test5DeleteNewDeviceDiffValueClonePopulatedSameControl2.log Concat - Test5DeleteNewDeviceDiffValueClonePopulatedSameControl3.log

Comment: Stopping troubleshooting to report to creator. It is very possible that during our troubleshooting of the issue, we mixed control on the same Parent/Child relationship and caused the above issue. Having both as Queueable seems to work fine. I did not test both as synchronous due to lack of time. That being said, I am unsure if this is a bug or expected. If the latter, might need to be documented.

jamessimone commented 3 months ago

@Silchuki14 thanks for all of the info you've included. I wanted to get the fix for #595 out there ASAP, I'll be taking a look at this issue and I'll let you know if I have any follow-up questions

jamessimone commented 3 months ago

@Silchuki14 some of the logs you attached only have an entry saying that rollup logging is not enabled. that being said, I might have enough info - the test 2 ones in particular are like that. That being said, I think my answer to this is a "it depends" - if your org has automations that can mix updates between both an asynchronous and sync code path, that could definitely lead to inconsistent results.

The possible exception to that would be if the Should Run Single Records Synchronously checkbox was toggled on your rollup control records, as that uses row locking for single record updates (which would force concurrent updates to "wait their turn"). Let me know if you think this is worth keeping open or not - at the moment, though, I think this isn't something I would be able to improve upon from within the package.

Silchuki14 commented 3 months ago

@jamessimone My bad. I guess Synchronous Rollup Control did not have logs enable. Happy to redo the test with them if you want more logs.

I kind of understand how the Should Run Single Records Synchronously works. Based on its name, do you recommend this be turned on on Synchronous Rollups? It is not right now anywhere, so I am tempted to try it out.

Thank you!

jamessimone commented 3 months ago

@Silchuki14 I would definitely try it out. It's a classic "it depends" answer though - in an automation-heavy org, using the FOR UPDATE clause (which that flag controls) can lead to row locks. It can help to alleviate issues like this, though: it's certainly worth trying out to see if that helps

jamessimone commented 2 months ago

Closing - please feel free to reach out if you feel the question hasn't been answered