⚡️ Avoid committing no-ops to the database

This change checks for no-ops just before committing, and avoids writing
them to the database.

Motivation
----------
Sometimes we have situations where multiple clients try to make an
identical change to the same document at the same time (if multiple
clients are trying to keep two documents in sync, for example).

When this happens, this can result in a `O(n^2)` retry cascade, since
the requests will all keep retrying, with only a single request
succeeding at a time, even if those ops are all no-ops.

Solution
--------
This change adds a new special `ERR_NO_OP` error code. If the server
detects a no-op, it will skip committing, and early-return this error
code, which takes the request out of the retry queue.

The client then has some special handling for this case, where it will:

 1. Fetch from remote to get up-to-date with any potential conflicting
    ops
 2. Then acknowledge the in-flight op, **without** bumping the version
    (since the op wasn't committed)

Note that if `$fixup()` has been used, we **do not** early-return a
no-op error, even if the fixup results in a no-op, since the client
would be left in an inconsistent state without the fixup ops being
returned and applied.

Author: alecgibson

docs/api/backend.md

@@ -113,6 +113,19 @@ Optional
 
 {: .d-inline-block }
 
+`doNotCommitNoOps` -- boolean
+
+Optional
+{: .label .label-grey }
+
+> Default: `false`
+
+> If set to `true`, will avoid committing no-ops to the database. Clients that submit no-ops
+> (or ops that are transformed to no-ops) will have their ops acknowledged as if they were
+> committed, but the doc version will not increment.
+
+{: .d-inline-block }
+
 `errorHandler` -- Function
 
 Optional

docs/api/sharedb-error.md

@@ -100,3 +100,9 @@ Representation of an error, with a machine-parsable [code](#error-codes).
 ### `ERR_TYPE_CANNOT_BE_PROJECTED`
 
 > The document's type cannot be projected. [`json0`]({{ site.baseurl }}{% link types/json0.md %}) is currently the only type that supports projections.
+
+### `ERR_NO_OP`
+
+> The submitted op resulted in a no-op, possibly after transformation by a remote op.
+
+> This is normal behavior and the client should swallow this error without bumping doc version.

lib/backend.js

@@ -43,6 +43,7 @@ function Backend(options) {
       'new Backend({doNotForwardSendPresenceErrorsToClient: true})\n\n'
     );
   }
+  this.doNotCommitNoOps = !!options.doNotCommitNoOps;
 
   // Map from event name to a list of middleware
   this.middleware = Object.create(null);

lib/client/doc.js

@@ -328,6 +328,15 @@ Doc.prototype._handleSubscribe = function(error, snapshot) {
 
 Doc.prototype._handleOp = function(err, message) {
   if (err) {
+    if (err.code === ERROR_CODE.ERR_NO_OP && message.seq === this.inflightOp.seq) {
+      // Our op was a no-op, either because we submitted a no-op, or - more
+      // likely - because our op was transformed into a no-op by the server
+      // because of a similar remote op. In this case, the server has avoided
+      // committing the op to the database, and we should just clear the in-flight
+      // op and call the callbacks. However, let's first catch ourselves up to
+      // the remote, so that we're in a nice consistent state
+      return this.fetch(this._clearInflightOp.bind(this));
+    }
     if (this.inflightOp) {
       return this._rollback(err);
     }

lib/error.js

@@ -35,6 +35,7 @@ ShareDBError.CODES = {
   ERR_MAX_SUBMIT_RETRIES_EXCEEDED: 'ERR_MAX_SUBMIT_RETRIES_EXCEEDED',
   ERR_MESSAGE_BADLY_FORMED: 'ERR_MESSAGE_BADLY_FORMED',
   ERR_MILESTONE_ARGUMENT_INVALID: 'ERR_MILESTONE_ARGUMENT_INVALID',
+  ERR_NO_OP: 'ERR_NO_OP',
   ERR_OP_ALREADY_SUBMITTED: 'ERR_OP_ALREADY_SUBMITTED',
   ERR_OP_NOT_ALLOWED_IN_PROJECTION: 'ERR_OP_NOT_ALLOWED_IN_PROJECTION',
   ERR_OP_SUBMIT_REJECTED: 'ERR_OP_SUBMIT_REJECTED',

lib/protocol.js

@@ -1,6 +1,6 @@
 module.exports = {
   major: 1,
-  minor: 1,
+  minor: 2,
   checkAtLeast: checkAtLeast
 };
 

lib/submit-request.js

@@ -2,6 +2,7 @@ var ot = require('./ot');
 var projections = require('./projections');
 var ShareDBError = require('./error');
 var types = require('./types');
+var protocol = require('./protocol');
 
 var ERROR_CODE = ShareDBError.CODES;
 
@@ -152,6 +153,18 @@ SubmitRequest.prototype.submit = function(callback) {
       err = request._transformOp(ops);
       if (err) return callback(err);
 
+      var skipNoOp = backend.doNotCommitNoOps &&
+        protocol.checkAtLeast(request.agent.protocol, '1.2') &&
+        request.op.op &&
+        request.op.op.length === 0;
+
+      if (skipNoOp) {
+        // The op is a no-op, either because it was submitted as such, or - more
+        // likely - because it was transformed into one. Let's avoid committing it
+        // and tell the client.
+        return callback(request.noOpError());
+      }
+
       if (op.v !== snapshot.v) {
         // This shouldn't happen, but is just a final sanity check to make
         // sure we have transformed the op to the current snapshot version
@@ -361,3 +374,9 @@ SubmitRequest.prototype.maxRetriesError = function() {
     'Op submit failed. Exceeded max submit retries of ' + this.maxRetries
   );
 };
+SubmitRequest.prototype.noOpError = function() {
+  return new ShareDBError(
+    ERROR_CODE.ERR_NO_OP,
+    'Op is a no-op. Skipping apply.'
+  );
+};

test/client/submit.js

@@ -1290,6 +1290,119 @@ module.exports = function() {
       });
     });
 
+    it('commits both of two identical ops submitted from different clients by default', function(done) {
+      var backend = this.backend;
+      backend.doNotCommitNoOps = false;
+      var doc1 = backend.connect().get('dogs', id);
+      var doc2 = backend.connect().get('dogs', id);
+      var op = [{p: ['tricks', 0], ld: 'fetch'}];
+
+      async.series([
+        doc1.create.bind(doc1, {tricks: ['fetch', 'sit']}),
+        doc2.fetch.bind(doc2),
+        async.parallel.bind(async.parallel, [
+          doc1.submitOp.bind(doc1, op),
+          doc2.submitOp.bind(doc2, op)
+        ]),
+        function(next) {
+          expect(doc1.data).to.eql({tricks: ['sit']});
+          expect(doc2.data).to.eql({tricks: ['sit']});
+          next();
+        },
+        function(next) {
+          backend.db.getOps('dogs', id, 0, null, {}, function(error, ops) {
+            if (error) return next(error);
+            // Expect:
+            // v0: create
+            // v1: update
+            // v2: duplicate update
+            expect(ops).to.have.length(3);
+            next();
+          });
+        }
+      ], done);
+    });
+
+    it('only commits one of two identical ops submitted from different clients', function(done) {
+      var backend = this.backend;
+      backend.doNotCommitNoOps = true;
+      var doc1 = backend.connect().get('dogs', id);
+      var doc2 = backend.connect().get('dogs', id);
+      var op = [{p: ['tricks', 0], ld: 'fetch'}];
+
+      async.series([
+        doc1.create.bind(doc1, {tricks: ['fetch', 'sit']}),
+        doc2.fetch.bind(doc2),
+        async.parallel.bind(async.parallel, [
+          doc1.submitOp.bind(doc1, op),
+          doc2.submitOp.bind(doc2, op)
+        ]),
+        function(next) {
+          expect(doc1.data).to.eql({tricks: ['sit']});
+          expect(doc2.data).to.eql({tricks: ['sit']});
+          next();
+        },
+        function(next) {
+          backend.db.getOps('dogs', id, 0, null, {}, function(error, ops) {
+            if (error) return next(error);
+            // Expect:
+            // v0: create
+            // v1: update
+            // no duplicate update
+            expect(ops).to.have.length(2);
+            next();
+          });
+        }
+      ], done);
+    });
+
+    it('can submit a new op after getting a no-op', function(done) {
+      var backend = this.backend;
+      backend.doNotCommitNoOps = true;
+      var doc1 = backend.connect().get('dogs', id);
+      var doc2 = backend.connect().get('dogs', id);
+      var op = [{p: ['tricks', 0], ld: 'fetch'}];
+
+      async.series([
+        doc1.create.bind(doc1, {tricks: ['fetch', 'sit']}),
+        doc2.fetch.bind(doc2),
+        async.parallel.bind(async.parallel, [
+          doc1.submitOp.bind(doc1, op),
+          doc2.submitOp.bind(doc2, op)
+        ]),
+        function(next) {
+          expect(doc1.data).to.eql({tricks: ['sit']});
+          expect(doc2.data).to.eql({tricks: ['sit']});
+          next();
+        },
+        doc1.submitOp.bind(doc1, [{p: ['tricks', 0], li: 'play dead'}]),
+        function(next) {
+          expect(doc1.data.tricks).to.eql(['play dead', 'sit']);
+          next();
+        }
+      ], done);
+    });
+
+    it('fixes up even if an op is fixed up to become a no-op', function(done) {
+      var backend = this.backend;
+      backend.doNotCommitNoOps = true;
+      var doc = backend.connect().get('dogs', id);
+
+      backend.use('apply', function(req, next) {
+        req.$fixup([{p: ['fixme'], od: true}]);
+        next();
+      });
+
+      async.series([
+        doc.create.bind(doc, {}),
+        doc.submitOp.bind(doc, [{p: ['fixme'], oi: true}]),
+        function(next) {
+          expect(doc.data).to.eql({});
+          next();
+        }
+      ], done);
+    });
+
     describe('type.deserialize', function() {
       it('can create a new doc', function(done) {
         var doc = this.backend.connect().get('dogs', id);