Fix for high concurrency issue

[billwashere]

This pull request does two changes

1. Changes the datatype on the ops and snapshots tables to JSONB

2. Attempts to resolve the high concurrency issue but committing the snapshot and operation in one transaction

[nornagon]

Thanks for this PR! The jsonb and connection pooling stuff looks great, but I'm not ready to merge the change to the commit query. If you want to split those other two changes out into a separate PR, I'll merge those, but if you'd rather work on the query too and merge it all together then that's fine too.

[nornagon]

can change this back :)

[nornagon]

Looks like the indentation is off here, please use 2-space indentation :)

[nornagon]

Just fold the data type back into the CREATE TABLE statements

[nornagon]

I think it's fine to use const directly here, especially since you're using string interpolation 2 lines below--this is ES6

[nornagon]

I found this query really hard to follow, even after reformatting and reindenting it. It's doing a lot of stuff, but I don't know if it's correct--could you explain (perhaps in a comment in the file) what this is doing and why?

Here's the reformatted version I made:

WITH snaps AS (
INSERT INTO snapshots (collection, doc_id, doc_type, version, data)
SELECT n.*FROM (SELECT $1 c, $2 d, $4 t, $3::integer v, $5::jsonb daa) n WHERE v = (
SELECT version+1 v
FROM snapshots
WHERE collection = $1AND doc_id = $2
FOR UPDATE
) OR NOT EXISTS (
SELECT1FROM snapshots
WHERE collection = $1AND doc_id = $2
FOR UPDATE
)
ON CONFLICT (collection, doc_id) DO UPDATESET version = $3, data = $5, doc_type = $4
RETURNING version
)
INSERT INTO ops (collection, doc_id, version, operation)
SELECT n.*FROM (SELECT $1 c, $2 t, $3::integer v, $6::jsonb daa) n WHERE (
v = (
SELECTmax(version)+1 v
FROM ops
WHERE collection = $1AND doc_id = $2
) OR NOT EXISTS (
SELECT1FROM ops
WHERE collection = $1AND doc_id = $2
FOR UPDATE
)
) AND EXISTS (SELECT1FROM snaps)
RETURNING version

[billwashere]

Yeah it non-trivial sql. I've now added the following comment

This query uses common table expression to upsert the snapshot table
(iff the new version is exactly 1 more than the latest table or if
the document id does not exists)

It will then Insert into the ops table if it is exactly 1 more than the
latest table or it the first operation and iff the previous insert into
the snapshot table is successful.

This result of this query the version of the newly inserted operation

If either the ops or the snapshot insert fails then 0 rows are returned

If 0 zeros are return then the callback must return false

[nornagon]

don't log anything from this library—that's up to the client.

[billwashere]

Hi @nornagon
I've split out pooling into a seperate change - #5

[billwashere]

I've this pull request with the required changes.

Additionally

1. Commit no longer requires jsonb, it works fine without it
2. On close, the connection pool drains

[jasoniangreen]

Hey @billwashere and @nornagon, what is the state of this? Do you need any help?

[billwashere]

I believe that it working correctly, however due to the fact that it is such a big change more testing may be required (I'll leave how much testing to @nornagon).

If you want to try it out and see if you can break it, that would be great.

[nornagon]

Would you be able to rebase this on top of master? It looks like mostly minor conflicts. Also retitle the PR, since this is now only addressing the concurrency issue.

Thanks! If this is reported working for a few people, I'm happy to merge it (at least, once I sit down and actually understand what's going on with the query...). The code as it is is not particularly well tested, so new untested code at least won't be a regression, but it'd be great to have better test coverage! especially cool would be if you could construct a test that fails with the old code but passes with the new.

[ianberdin]

@billwashere can you merge this to master and update npm package?

[Utwo]

@billwashere can you merge this to master and update npm package?

I think you can use this package: https://github.com/plotdb/sharedb-postgres

[nornagon]

if @zbryikt or someone wants to take over maintenance of this repo, I'm happy to grant access.