dss icon indicating copy to clipboard operation
dss copied to clipboard

Reduce likelihood of contention in transactions

Open BenjaminPelletier opened this issue 3 years ago • 2 comments

Currently, there are circumstances when many transactions may fundamentally contend to change the same data fields. Specifically, if there are many flights being updated in the same area at the same time, the notification indices for each flight's associated subscription will need to be updated for each change to a flight in the area. We can reduce the likelihood of a deadlock or poor database behavior in two ways:

  1. Always sort ID parameters before passing them in a SQL command since they will be evaluated in series rather than parallel
  2. Add a "for update" flag when reading database entities to use a SELECT FOR UPDATE SQL command where appropriate

BenjaminPelletier avatar Sep 21 '22 00:09 BenjaminPelletier

Notes regarding early locking with SELECT FOR UPDATE:

Early ISA subscription locking

Interactions with NetRID's ISAs might benefit from having a method like SCD's LockSubscriptionsOnCells for RID subscriptions. Ie, the cells for which rid subscriptions are updated with UpdateNotificationIdxInCells could be locked early.

Notes

An alternative to having an early locking method would be to run the index-incrementing query earlier when certain operations are done. For example, in InsertISA or UpdateISA, once the query is determined to be valid, UpdateNotificationIdxsInCells could be called before the query deleting or mutating the ISA.

We could also try this approach with the OIR handlers: instead of calling LockSubscriptionsOnCells we could directly call the query incrementing indices?

Here I am assuming that locking records and actually updating them has a comparable cost: if it is the case, we might be able to save a round-trip to CRDB and make OIR upserts even faster?

Early subscription locking for OIR deletion

As a sidenote, for OIRs, LockSubscriptionsOnCells is only called for OIR creation or mutation, but not deletion. Assuming this is an omission, we could add such a locking step around here, after the OIR has been fetched and its cells are available. This is covered in #1113

Shastick avatar Sep 12 '24 14:09 Shastick

@BenjaminPelletier, before I go one with adding optional "FOR UPDATE" to various requests, may I ask you to check https://github.com/interuss/dss/pull/1117 and confirm that this is the approach you were thinking off?

Shastick avatar Sep 12 '24 15:09 Shastick