INVALID_ID_FIELD error when RollupJob deletes schedule items concurrently with trigger upsert

[s-kikuchi-safie]

GitHub Issue Draft for DLRS

Title

INVALID_ID_FIELD error when RollupJob deletes schedule items concurrently with trigger upsert

---

Issue Body

Description

When using Scheduled calculation mode, we occasionally encounter INVALID_ID_FIELD errors during the upsert of LookupRollupSummaryScheduleItems__c records. This occurs when the RollupJob batch is deleting schedule items at the same time a trigger is attempting to upsert new ones with the same QualifiedParentID__c.

Error Message

System.DmlException: Upsert failed. First exception on row 0; first error: INVALID_ID_FIELD, invalid record id: ()
(dlrs)
Trigger.dlrs_ReceiptDeliveryTrigger: line 7, column 1

Suspected Root Cause

We suspect this race condition may occur with the following sequence:

1. T0: RollupJob starts deleting a schedule item (record may be locked)
2. T1: Trigger executes upsert with same QualifiedParentID__c
   • Search by external ID finds the record (not yet deleted)
   • Attempts update, waits for lock
3. T2: RollupJob commits deletion (record physically deleted)
4. T3: Trigger's lock wait completes
   • Attempts to update a now-deleted record ID
   • Result: INVALID_ID_FIELD error

This appears to be a timing issue between the deletion process and the upsert operation.

Current Code Behavior

In RollupService.cls lines 1342-1361, only DUPLICATE_VALUE errors are ignored:

// iterate through errors, only return an exception for something other than DUPLICATE_VALUE
// because DUPLICATE_VALUE errors are usually sharing problems and we're happy as long as the record is in the database
for (Integer i = 0, j = results.size(); i < j; i++) {
  if (!results[i].isSuccess()) {
    for (Database.Error err : results[i].getErrors()) {
      if (err.getStatusCode() != System.StatusCode.DUPLICATE_VALUE) {
        throw new DmlException(
          'Upsert failed. First exception on row ' + i +
          '; first error: ' + err.getStatusCode() + ', ' +
          err.getMessage() + ': ' + err.getFields()
        );
      }
    }
  }
}

Proposed Solution

Treat INVALID_ID_FIELD the same as DUPLICATE_VALUE - ignore it because:

1. It's a transient race condition: The schedule item will be recreated on the next trigger execution
2. The parent record is already processed: If RollupJob deleted the item, it has already calculated the rollup
3. Same logic as DUPLICATE_VALUE: As long as rollup processing occurs, the specific schedule item persistence doesn't matter

Suggested Code Change

for (Database.Error err : results[i].getErrors()) {
  if (err.getStatusCode() != System.StatusCode.DUPLICATE_VALUE
      && err.getStatusCode() != System.StatusCode.INVALID_ID_FIELD) {  // Add this
    throw new DmlException(...);
  }
}

Thank you for considering this issue!

[aheber]

@s-kikuchi-safie thank you very much for digging into this and providing a proposed change.

Do you have some way to test this behavior on-demand? I'd love to be able to replicate it and demonstrate that it is what you expect and that your fix works as desired.

Assuming this is correct, it seems to open a chance that the old record is deleted and the new record is not added. Leaving behind nothing to cause the parent to be recalculated and leaving a stale value. You mention it will be recalculated on the next update but until then (which could be weeks or months later) the parent record might have an incorrect value.

The reason I felt safer about adding the DUPLICATE_VALUE check is because it indicated the record was still queued for calculation, it was just a security condition that prevented the submitting transaction from seeing the record. We would still get parent calculation on the next transaction. Your situation is kinda the opposite. DLRS is trying to enqueue a record but that isn't working so the update is going to be lost.

I hope that concern makes sense. If we can replicate this scenario than we can consider something like a retry or other logic to attempt to ensure that record isn't lost.

[s-kikuchi-safie]

@aheber
Thank you for the thoughtful response and raising this important concern.

Regarding the "lost update" concern

You're absolutely right. In my use case, I had configured the RollupJob to run every 5 minutes, so I didn't fully consider the scenario where the parent record could remain stale for weeks or months. That's a valid concern that I overlooked. Simply ignoring INVALID_ID_FIELD errors would indeed risk leaving parent records with incorrect values until the next child record update.

Regarding on-demand testing

Unfortunately, reproducing this issue on-demand is quite difficult. The error occurs only when:

1. The RollupJob is actively deleting a schedule item
2. A trigger attempts to upsert a schedule item with the same QualifiedParentID__c at exactly the same moment

This timing window is very narrow (likely just a few seconds), making it hard to reliably trigger in a controlled test environment. The issue was identified by correlating error timestamps with the RollupJob execution schedule.

Proposed alternative: Retry logic

Instead of simply ignoring the error, perhaps a retry mechanism would be more appropriate:

for (Database.Error err : results[i].getErrors()) {
  if (err.getStatusCode() == System.StatusCode.INVALID_ID_FIELD) {
    // The record may have been deleted by RollupJob during our upsert attempt
    // Wait briefly and retry the upsert for this specific item
    retryItems.add(scheduledItems[i]);
  } else if (err.getStatusCode() != System.StatusCode.DUPLICATE_VALUE) {
    throw new DmlException(...);
  }
}

// Retry failed items after a brief delay
if (!retryItems.isEmpty()) {
  Database.upsert(retryItems, LookupRollupSummaryScheduleItems__c.QualifiedParentID__c, false, AccessLevel.SYSTEM_MODE);
}

This approach would:

1. Catch the INVALID_ID_FIELD error (indicating the record was deleted during our operation)
2. Retry the upsert, which should now succeed as an insert since the conflicting record is gone
3. Ensure the schedule item is properly queued for parent recalculation

Would this approach address the concern about lost updates while still handling the race condition gracefully?

[aheber]

@s-kikuchi-safie I've been working on this and trying to find a solution. I have a very isolated test framework setup. This doesn't use any DLRS code but I'm happy to share if you'd like.

In this model I have two custom objects:

1. The "upsert target" object. This is what I'm writing to and deleting from.
2. I have a log object so I can record database errors without crashing the transaction.

I'm using two queuables that I start at the same time. One of them creates records and the other finds any records on that object and deletes them. I then let the queuables self-chain to a configured limit (5 or 10 is usually enough to experience the conflict).

So in this design, usually at least once per run I can force the error you've described above. Trying to upsert against a record that was recently deleted, the DML throwing "INVALID_ID_FIELD" as a result. I also sometimes get an UNABLE_TO_LOCK_ROW error for the same reason.

Now that we have a test harness I can play with different strategies for forcing the update to succeed. The easiest was to simply retry the upsert as you've suggested. This still doesn't entirely guarantee the database is in a synchronized state but it is better than nothing. I think we can get that logic added so it tries twice if that is the only error. (If there are other errors then it will throw as it does today)

With this solution still being a bandaid I think we'll need a long-term solution for handling this situation. The plan is to eventually handle writing scheduled item records using Platform Events so we can capture the need without consuming DML rows. We have many people who run into trouble because just writing the scheduled item records would exceed their 10k DML row limit. It can be tricky to find the balance and emitting platform events would likely be a great middle ground to help ensure things don't get lost.

I'll keep you updated on our efforts for the short-term bandaid as well as the long term fix.