From 1ed2925c10710a653a471ba6d41ff8f1554e01a6 Mon Sep 17 00:00:00 2001 From: William Claydon Date: Sat, 10 Mar 2018 12:28:19 +1100 Subject: [PATCH 1/9] JSONB, PG library, and check max update --- README.md | 47 +++++++++++++++++-- index.js | 124 +++++++++++++++++--------------------------------- package.json | 2 +- structure.sql | 10 ++++ 4 files changed, 96 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index b006df0..794ed3b 100644 --- a/README.md +++ b/README.md @@ -8,15 +8,56 @@ Doesn't support queries (yet?). Moderately experimental. (This drives [Synaptograph](https://www.synaptograph.com)'s backend, and [@nornagon](https://github.com/nornagon) hasn't noticed any issues so far.) +## Requirements + +Due to the fix to resolve [high concurency issues](https://github.com/share/sharedb-postgres/issues/1) Postgres 9.5+ is now required. + +## Migrating older versions + +Older versions of this adaptor used the data type json. You will need to alter the data type prior to using if you are upgrading. + +```PLpgSQL +ALTER TABLE ops + ALTER COLUMN operation + SET DATA TYPE jsonb + USING operation::jsonb; + +ALTER TABLE snapshots + ALTER COLUMN data + SET DATA TYPE jsonb + USING data::jsonb; +``` + ## Usage -`sharedb-postgres` wraps native [node-postgres](https://github.com/brianc/node-postgres), and it supports the same configuration options. +`sharedb-postgres-jsonb` wraps native [node-postgres](https://github.com/brianc/node-postgres), and it supports the same configuration options. To instantiate a sharedb-postgres wrapper, invoke the module and pass in your -PostgreSQL configuration as an argument. For example: +PostgreSQL configuration as an argument or use environmental arguments. + +For example using environmental arugments: + +```js +var db = require('sharedb-postgres')(); +var backend = require('sharedb')({db: db}) +``` + +Then executing via the command line + +``` +PGUSER=dbuser PGPASSWORD=secretpassword PGHOST=database.server.com PGDATABASE=mydb PGPORT=5433 npm start +``` + +Example using an object ```js -var db = require('sharedb-postgres')('postgres://localhost/mydb'); +var db = require('sharedb-postgres')({ + user: 'dbuser', + host: 'database.server.com', + database: 'mydb', + password: 'secretpassword', + port: 5433, +}); var backend = require('sharedb')({db: db}) ``` diff --git a/index.js b/index.js index 3f7dc57..9d8359e 100644 --- a/index.js +++ b/index.js @@ -10,6 +10,7 @@ function PostgresDB(options) { this.closed = false; this.pg_config = options; + this.pool = new pg.Pool(this.pg_config) }; module.exports = PostgresDB; @@ -20,11 +21,6 @@ PostgresDB.prototype.close = function(callback) { if (callback) callback(); }; -function rollback(client, done) { - client.query('ROLLBACK', function(err) { - return done(err); - }) -} // Persists an op and snapshot if it is for the next version. Calls back with // callback(err, succeeded) @@ -39,91 +35,53 @@ PostgresDB.prototype.commit = function(collection, id, op, snapshot, options, ca * } * snapshot: PostgresSnapshot */ - pg.connect(this.pg_config, function(err, client, done) { - if (err) { - done(client); - callback(err); - return; - } - function commit() { - client.query('COMMIT', function(err) { - done(err); - if (err) { - callback(err); - } else { - callback(null, true); - } - }) + this.pool.connect((err, client, done) => { + if (err) { + done(client); + callback(err); + return; + } + /*const*/ var query = { + name: 'sdb-commit-op-and-snap', + text: `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 = $1 and doc_id = $2 for update) or not exists (select 1 from snapshots where collection = $1 and doc_id = $2 for update) + On conflict(collection, doc_id) do update set 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 = (select max(version)+1 v from ops where collection = $1 and doc_id = $2) or not exists (select 1 from ops where collection = $1 and doc_id = $2 for update)) and exists (select 1 from snaps) + Returning version`, + values: [collection,id,snapshot.v, snapshot.type, snapshot.data,op] } - client.query( - 'SELECT max(version) AS max_version FROM ops WHERE collection = $1 AND doc_id = $2', - [collection, id], - function(err, res) { - var max_version = res.rows[0].max_version; - if (max_version == null) - max_version = 0; - if (snapshot.v !== max_version + 1) { - return callback(null, false); - } - client.query('BEGIN', function(err) { - client.query( - 'INSERT INTO ops (collection, doc_id, version, operation) VALUES ($1, $2, $3, $4)', - [collection, id, snapshot.v, op], - function(err, res) { - if (err) { - // TODO: if err is "constraint violation", callback(null, false) instead - rollback(client, done); - callback(err); - return; - } - if (snapshot.v === 1) { - client.query( - 'INSERT INTO snapshots (collection, doc_id, doc_type, version, data) VALUES ($1, $2, $3, $4, $5)', - [collection, id, snapshot.type, snapshot.v, snapshot.data], - function(err, res) { - // TODO: - // if the insert was successful and did insert, callback(null, true) - // if the insert was successful and did not insert, callback(null, false) - // if there was an error, rollback and callback(error) - if (err) { - rollback(client, done); - callback(err); - return; - } - commit(); - } - ) - } else { - client.query( - 'UPDATE snapshots SET doc_type = $3, version = $4, data = $5 WHERE collection = $1 AND doc_id = $2 AND version = ($4 - 1)', - [collection, id, snapshot.type, snapshot.v, snapshot.data], - function(err, res) { - // TODO: - // if any rows were updated, success - // if 0 rows were updated, rollback and not success - // if error, rollback and not success - if (err) { - rollback(client, done); - callback(err); - return; - } - commit(); - } - ) - } - } - ) - }) + client.query(query, (err, res) => { + if (err) { + console.log(err.stack) + callback(err) + } else if(res.rows.length === 0) { + done(client); + console.log("Unable to commit, not the latest version") + callback(null,false) + } + else { + done(client); + console.log(res.rows[0]) + callback(null,true) } - ) - }) + }) + + }) }; // Get the named document from the database. The callback is called with (err, // snapshot). A snapshot with a version of zero is returned if the docuemnt // has never been created in the database. PostgresDB.prototype.getSnapshot = function(collection, id, fields, options, callback) { - pg.connect(this.pg_config, function(err, client, done) { + this.pool.connect(function(err, client, done) { if (err) { done(client); callback(err); @@ -173,7 +131,7 @@ PostgresDB.prototype.getSnapshot = function(collection, id, fields, options, cal // // Callback should be called as callback(error, [list of ops]); PostgresDB.prototype.getOps = function(collection, id, from, to, options, callback) { - pg.connect(this.pg_config, function(err, client, done) { + this.pool.connect(function(err, client, done) { if (err) { done(client); callback(err); diff --git a/package.json b/package.json index 3b1f446..f8863d3 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "keywords": ["sharedb", "sharejs", "share", "postgres"], "repository": "share/sharedb-postgres", "dependencies": { - "pg": "^4.5.1", + "pg": "^7.4.1", "sharedb": "^1.0.0-beta.7" } } diff --git a/structure.sql b/structure.sql index 606b5fc..fa9ec40 100644 --- a/structure.sql +++ b/structure.sql @@ -14,3 +14,13 @@ CREATE TABLE snapshots ( data json not null, PRIMARY KEY (collection, doc_id) ); + +ALTER TABLE ops + ALTER COLUMN operation + SET DATA TYPE jsonb + USING operation::jsonb; + +ALTER TABLE snapshots + ALTER COLUMN data + SET DATA TYPE jsonb + USING data::jsonb; \ No newline at end of file From 698bb625fc38dda547289585b57cb42c391baf86 Mon Sep 17 00:00:00 2001 From: William Claydon Date: Sat, 17 Mar 2018 18:06:53 +1100 Subject: [PATCH 2/9] typo --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 0293110..9f2ef25 100644 --- a/index.js +++ b/index.js @@ -50,7 +50,7 @@ PostgresDB.prototype.commit = function(collection, id, op, snapshot, options, ca * (iff the new version is exactly 1 more than the latest table or if * the document id does not exists) * - * It will than Insert into the ops table if it is exactly 1 more than the + * 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. * From 9e33580be093400c2ca5672d1b2f3104b8e1657d Mon Sep 17 00:00:00 2001 From: ZechyW Date: Wed, 28 Mar 2018 21:31:06 -0700 Subject: [PATCH 3/9] Bugfixes - In .commit(), the version for the op and version for the snapshot are different. We should use `op.v` instead of `snapshot.v` when committing the op. - In .getOps(), rows are sometimes returned out of order. Adding an explicit `ORDER BY` clause to the SQL query fixes this. --- index.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 442bd84..fe408f4 100644 --- a/index.js +++ b/index.js @@ -68,9 +68,11 @@ PostgresDB.prototype.commit = function(collection, id, op, snapshot, options, ca return callback(null, false); } client.query('BEGIN', function(err) { + // ZW: Snapshot version is always 1 above op version. We should + // use op.v here, not snapshot.v client.query( 'INSERT INTO ops (collection, doc_id, version, operation) VALUES ($1, $2, $3, $4)', - [collection, id, snapshot.v, op], + [collection, id, op.v, op], function(err, res) { if (err) { // TODO: if err is "constraint violation", callback(null, false) instead @@ -181,8 +183,11 @@ PostgresDB.prototype.getOps = function(collection, id, from, to, options, callba callback(err); return; } + + // ZW: Add explicit row ordering here client.query( - 'SELECT version, operation FROM ops WHERE collection = $1 AND doc_id = $2 AND version >= $3 AND version < $4', + 'SELECT version, operation FROM ops WHERE collection = $1 AND doc_id =' + + ' $2 AND version >= $3 AND version < $4 ORDER BY version ASC', [collection, id, from, to], function(err, res) { done(); From 3d8db1c70b03511d54a7a8dbed38786cbbbf7bc7 Mon Sep 17 00:00:00 2001 From: ZechyW Date: Fri, 30 Mar 2018 16:12:32 -0700 Subject: [PATCH 4/9] Bugfix - For the max version check in .commit(), we should query the snapshot table, not the ops table. (Op versions start from 0, whereas snapshot versions start from 1) --- index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index fe408f4..6f92739 100644 --- a/index.js +++ b/index.js @@ -57,8 +57,13 @@ PostgresDB.prototype.commit = function(collection, id, op, snapshot, options, ca } }) } + // ZW: Op versions start from 0, snapshot versions start from 1. To get + // the next snapshot version, we should query the snapshots table, not + // the ops table. + // (cf: the MemoryDB adapter, which uses the number of op rows, rather + // than the highest op version) client.query( - 'SELECT max(version) AS max_version FROM ops WHERE collection = $1 AND doc_id = $2', + 'SELECT max(version) AS max_version FROM snapshots WHERE collection = $1 AND doc_id = $2', [collection, id], function(err, res) { var max_version = res.rows[0].max_version; From 62bc7da9509f8ff0c722b9743661d6fe23a7389b Mon Sep 17 00:00:00 2001 From: ZechyW Date: Wed, 11 Apr 2018 18:08:16 -0700 Subject: [PATCH 5/9] Modifying version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index cbf8147..e9363cb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharedb-postgres", - "version": "3.0.0", + "version": "3.0.0-zw.1", "description": "PostgreSQL adapter for ShareDB", "main": "index.js", "scripts": { From dbdfbedb75e6608605cf2aaf3c8791723dbc8024 Mon Sep 17 00:00:00 2001 From: ZechyW Date: Mon, 23 Jul 2018 02:30:31 +0800 Subject: [PATCH 6/9] Re-adding op version bugfix --- index.js | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 7aee016..aca58e9 100644 --- a/index.js +++ b/index.js @@ -60,6 +60,9 @@ PostgresDB.prototype.commit = function (collection, id, op, snapshot, options, c * Casting is required as postgres thinks that collection and doc_id are * not varchar */ + // ZW: We also should have the first op version be 0, not 1, to match the + // reference MemoryDB implementation. Catching up outdated clients who + // reconnect may be buggy otherwise. const query = { name: "sdb-commit-op-and-snap", text: `WITH snapshot_id AS ( @@ -91,7 +94,7 @@ PostgresDB.prototype.commit = function (collection, id, op, snapshot, options, c FROM ops WHERE collection = $1 AND doc_id = $2 ) - ) AND EXISTS (SELECT 1 FROM snapshot_id) + ) AND EXISTS (SELECT 0 FROM snapshot_id) RETURNING version`, values: [collection, id, snapshot.v, snapshot.type, snapshot.data, op] }; diff --git a/package.json b/package.json index b359b3f..8d4dae0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharedb-postgres", - "version": "3.0.0-zw.1", + "version": "3.0.0-zw.2", "description": "PostgreSQL adapter for ShareDB", "main": "index.js", "scripts": { From e2acfc0e4c82480b98e67db311270c864ce588c2 Mon Sep 17 00:00:00 2001 From: ZechyW Date: Mon, 23 Jul 2018 02:37:01 +0800 Subject: [PATCH 7/9] Typo in last commit --- index.js | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index aca58e9..0f7399e 100644 --- a/index.js +++ b/index.js @@ -90,11 +90,11 @@ PostgresDB.prototype.commit = function (collection, id, op, snapshot, options, c FROM ops WHERE collection = $1 AND doc_id = $2 ) OR NOT EXISTS ( - SELECT 1 + SELECT 0 FROM ops WHERE collection = $1 AND doc_id = $2 ) - ) AND EXISTS (SELECT 0 FROM snapshot_id) + ) AND EXISTS (SELECT 1 FROM snapshot_id) RETURNING version`, values: [collection, id, snapshot.v, snapshot.type, snapshot.data, op] }; diff --git a/package.json b/package.json index 8d4dae0..8dcfa74 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharedb-postgres", - "version": "3.0.0-zw.2", + "version": "3.0.0-zw.3", "description": "PostgreSQL adapter for ShareDB", "main": "index.js", "scripts": { From 57600c83cb0fe767e8c22f122d810685fbaac5d0 Mon Sep 17 00:00:00 2001 From: ZechyW Date: Mon, 23 Jul 2018 02:54:58 +0800 Subject: [PATCH 8/9] ACTUALLY fix the bug this time --- index.js | 28 +++++++++++++++++----------- package.json | 2 +- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 0f7399e..19ed421 100644 --- a/index.js +++ b/index.js @@ -60,16 +60,18 @@ PostgresDB.prototype.commit = function (collection, id, op, snapshot, options, c * Casting is required as postgres thinks that collection and doc_id are * not varchar */ - // ZW: We also should have the first op version be 0, not 1, to match the - // reference MemoryDB implementation. Catching up outdated clients who - // reconnect may be buggy otherwise. + // ZW: We also should have the first op version be 0 (the actual value + // of op.v) instead of 1, in order to match the reference MemoryDB + // implementation. Catching up outdated clients who reconnect may be + // buggy otherwise. const query = { name: "sdb-commit-op-and-snap", text: `WITH snapshot_id AS ( INSERT INTO snapshots (collection, doc_id, doc_type, version, data) - SELECT $1::varchar collection, $2::varchar doc_id, $4 doc_type, $3 v, $5 d - WHERE $3 = ( - SELECT version+1 v + SELECT $1::varchar collection, $2::varchar doc_id, + $3 snap_type, $4 snap_v, $5 snap_data + WHERE $4 = ( + SELECT version+1 snap_v FROM snapshots WHERE collection = $1 AND doc_id = $2 FOR UPDATE @@ -79,24 +81,28 @@ PostgresDB.prototype.commit = function (collection, id, op, snapshot, options, c WHERE collection = $1 AND doc_id = $2 FOR UPDATE ) - ON CONFLICT (collection, doc_id) DO UPDATE SET version = $3, data = $5, doc_type = $4 + ON CONFLICT (collection, doc_id) DO + UPDATE SET doc_type = $3, version = $4, data = $5 RETURNING version ) INSERT INTO ops (collection, doc_id, version, operation) - SELECT $1::varchar collection, $2::varchar doc_id, $3 v, $6 operation + SELECT $1::varchar collection, $2::varchar doc_id, + $6 op_v, $7 op WHERE ( - $3 = ( + $6 = ( SELECT max(version)+1 FROM ops WHERE collection = $1 AND doc_id = $2 ) OR NOT EXISTS ( - SELECT 0 + SELECT 1 FROM ops WHERE collection = $1 AND doc_id = $2 ) ) AND EXISTS (SELECT 1 FROM snapshot_id) RETURNING version`, - values: [collection, id, snapshot.v, snapshot.type, snapshot.data, op] + values: [ + collection, id, snapshot.type, snapshot.v, snapshot.data, op.v, op + ] }; client.query(query, (err, res) => { if (err) { diff --git a/package.json b/package.json index 8dcfa74..2a9415e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharedb-postgres", - "version": "3.0.0-zw.3", + "version": "3.0.1-zw", "description": "PostgreSQL adapter for ShareDB", "main": "index.js", "scripts": { From 50c5f21b5a0182ac6b5cff27482edb9b68f5d371 Mon Sep 17 00:00:00 2001 From: ZechyW Date: Tue, 24 Jul 2018 05:04:35 +0800 Subject: [PATCH 9/9] Updated readme/table structure for deletions - ShareDB sets `doc_type` and `data` to null in the `snapshots` table when a document is deleted --- README.md | 12 ++++++++++++ package.json | 2 +- structure.sql | 4 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 24d81ee..d197b88 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,18 @@ ALTER TABLE snapshots USING data::jsonb; ``` +In addition, the `snapshots` table might have a `NOT NULL` constraint on the `doc_type` and `data` columns that will need to be dropped, since these are set to null when a ShareDB document is deleted via its `.del()` method. + +```PLpgSQL +ALTER TABLE snapshots + ALTER COLUMN doc_type + DROP NOT NULL; + +ALTER TABLE snapshots + ALTER COLUMN data + DROP NOT NULL; +``` + ## Usage `sharedb-postgres` wraps native [node-postgres](https://github.com/brianc/node-postgres), and it supports the same configuration options. diff --git a/package.json b/package.json index 2a9415e..12996e7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sharedb-postgres", - "version": "3.0.1-zw", + "version": "3.0.2-zw", "description": "PostgreSQL adapter for ShareDB", "main": "index.js", "scripts": { diff --git a/structure.sql b/structure.sql index 977988d..66906d8 100644 --- a/structure.sql +++ b/structure.sql @@ -9,8 +9,8 @@ CREATE TABLE ops ( CREATE TABLE snapshots ( collection character varying(255) not null, doc_id character varying(255) not null, - doc_type character varying(255) not null, + doc_type character varying(255), -- Will be set to null if deleted version integer not null, - data jsonb not null, + data jsonb, -- Will be set to null if deleted PRIMARY KEY (collection, doc_id) );