share / sharedb-postgres

PostgreSQL adapter for ShareDB
MIT License
46 stars 22 forks source link

Max version check is wrong with high concurrency #1

Closed jclem closed 6 years ago

jclem commented 7 years ago

With lots of operations coming in from multiple sites, it is possible in this adapter to commit an operation with a version that has already been committed.

Given a scenario where User A and User B concurrently submit operation version 2, the max version check here will pass for both operations, and one operation will not be rebased against the one before it.

In order to resolve this, you can move the max version check inside of your transaction and append FOR UPDATE to the max version query:

SELECT max(version) AS max_version FROM ops WHERE collection = $1 AND doc_id = $2 FOR UPDATE

This will create a lock on this row for the duration of the transaction, ensuring that the later operation's max version check query blocks until the previous operation is fully committed.

I'm not actually using this adapter because my schema is different from what sharedb-postgres assumes, but I just spent a lot of time debugging this, and hopefully it can help you, too!

brainkim commented 6 years ago

@jclem I implemented row-locking in a knex port here (https://github.com/brainkim/sharedb-knex/blob/master/index.js#L35-L70). You can’t use for update in queries with aggregate functions (in postgres at least) so I moved the for update to a separate select. I have no idea what I'm doing when it comes to databases but it feels like this should fix the issue you describe, right? :3

jclem commented 6 years ago

@brainkim I’m not involved with this project anymore, so I can’t offer much guidance here. If it helps, I can point you to my old company’s lib we used to commit ShareDB operations: https://github.com/usecanvas/realtime-v2/blob/master/sharedb/lib/sharedb-postgres-canvas.js

Sorry I can’t be of more help. Good luck!