ivarsru5 / OrderInnService2.0

0 stars 0 forks source link

Models: Potential race condition for kitchen status updates #11

Open paulsnar opened 3 years ago

paulsnar commented 3 years ago

As it stands right now, if the kitchen were to want to notify the waiter that a part of their order has been prepared, the kitchen would update the Order document, replacing its parts array with one which contains updated entries with the done flag set to true where applicable.

This is actually a problem because it presents the opportunity for a race condition. If the waiter has appended a new order part with arrayUnion atomically, but the kitchen hasn't fetched this new part yet before submitting the replaced parts array, the new part will thereby be removed.

I haven't yet researched whether and how Firestore handles update conflicts like this. I have the slight feeling that it doesn't and this will essentially make either one of the updates win and the latter be dropped.

Due to the potential for data loss, this is a must before even launching the MVP. I have no clean solution as of now, so consider this ticket an open space for discussion of potential solutions.

paulsnar commented 3 years ago

Appears it's not as grim as I made it out to be. If transactions are performed within a server-side library, it takes out a lock on the documents, therefore presenting a chance for serialising operations: https://firebase.google.com/docs/firestore/transaction-data-contention

This will require moving all Order update operations into Cloud Functions though and doing none of them locally, which is inching ever-so-closer to just having our own backend again.

paulsnar commented 3 years ago

I decided to not go with Cloud Functions given that they require enabling billing for the project and cba.

When implemented, sending the update via minibackend will do the update in a transaction which ensures no race conditions are possible after the document read.

Keeping this open as a reminder to actually do this.