diff --git a/docs/middleware/actions.md b/docs/middleware/actions.md index 3d34dc6a..a928de2e 100644 --- a/docs/middleware/actions.md +++ b/docs/middleware/actions.md @@ -223,6 +223,16 @@ An operation was applied to a snapshot, and is about to be committed to the data This action has the same additional `context` properties as [`'submit'`](#submit). +## `'retry'` + +An operation lost the race to commit, because another op was committed to the same document first. The op is about to be retried: the snapshot will be fetched again, and the op transformed against the newer ops before passing through [`'apply'`](#apply) and [`'commit'`](#commit) again. [`'submit'`](#submit) is not triggered again. + +This is the place to reset any state that middleware stored on the request during the failed attempt. + +The `snapshot` and `channels` are still those of the failed attempt, so the `snapshot` already has the op applied. The snapshot is fetched again and the channels reset after this action, before [`'apply'`](#apply). + +This action has the same additional `context` properties as [`'submit'`](#submit). + ## `'afterWrite'` An operation and its updated snapshot were successfully written to the database. diff --git a/docs/middleware/op-submission.md b/docs/middleware/op-submission.md index 10f6e2b0..b6ef7315 100644 --- a/docs/middleware/op-submission.md +++ b/docs/middleware/op-submission.md @@ -18,6 +18,7 @@ When an op is submitted, it will pass through a number of middleware hooks as it - [**Submit**](#submit) -- an op has been received by the server - [**Apply**](#apply) -- an op is about to be applied to the snapshot - [**Commit**](#commit) -- an op and its updated snapshot are about to be committed to the database + - [**Retry**](#retry) -- an op lost the race to commit, and is about to be retried against a newer snapshot - [**After write**](#after-write) -- an op and its updated snapshot have successfully been committed to the database - [**Submit request end**](#submit-request-end) -- an op submission has finished (this is an _event_, **not** a middleware hook) @@ -66,7 +67,7 @@ backend.use('apply', (context, next) => { ``` {: .warn :} -The `'apply'` hook may be triggered more than once for a single submission. If another client wins the race to commit, the op is transformed over the winning op and applied again to the newer snapshot, re-triggering the hook. Be careful, therefore, with any side effects that assume the hook only runs once per op. +The `'apply'` hook may be triggered more than once for a single submission. If another client wins the race to commit, the op is transformed over the winning op and applied again to the newer snapshot, re-triggering the hook. Be careful, therefore, with any side effects that assume the hook only runs once per op. Use the [retry](#retry) hook to reset any state stored on the `context` during the failed attempt. ### Commit @@ -88,6 +89,28 @@ backend.use('commit', (context, next) => { }) ``` +### Retry + +The [`retry`]({{ site.baseurl }}{% link middleware/actions.md %}#retry) hook is triggered when another client wins the race to commit, before the op is retried. The op then passes through the [apply](#apply) and [commit](#commit) hooks again, against the newer snapshot. The [submit](#submit) hook is not triggered again. + +The retry reuses the same `context`, so any properties set on it during the failed attempt are still there. This hook is the place to reset them. + +In the example below, several `'apply'` middlewares share one lazily-taken snapshot clone, so only the first one that needs it pays for the copy. Without a reset, the retry would keep the failed attempt's clone. A plain `=` assignment, as in [Comparing old snapshot version with new version](#comparing-old-snapshot-version-with-new-version), re-captures on every attempt and doesn't need this hook. + +```js +backend.use('apply', (context, next) => { + // Any of several 'apply' middlewares may take the clone first + context.snapshotBeforeApply ??= structuredClone(context.snapshot) + next() +}) + +backend.use('retry', (context, next) => { + // Capture the newer snapshot on the next attempt + delete context.snapshotBeforeApply + next() +}) +``` + ### After write The [`afterWrite`]({{ site.baseurl }}{% link middleware/actions.md %}#afterwrite) hook is triggered after the op and updated snapshot have been successfully written to the database. This is the earliest that you know the op and snapshot are canonical. diff --git a/lib/backend.js b/lib/backend.js index 17a409df..ca105edf 100644 --- a/lib/backend.js +++ b/lib/backend.js @@ -89,6 +89,8 @@ Backend.prototype.MIDDLEWARE_ACTIONS = { receivePresence: 'receivePresence', // About to send presence information to a client sendPresence: 'sendPresence', + // An operation lost the race to commit and is about to be retried against a newer snapshot + retry: 'retry', // An operation is about to be submitted to the database submit: 'submit' }; diff --git a/lib/submit-request.js b/lib/submit-request.js index d2cac8d9..e9145f32 100644 --- a/lib/submit-request.js +++ b/lib/submit-request.js @@ -262,7 +262,11 @@ SubmitRequest.prototype.retry = function(callback) { } this._resetFixups(); this.backend.emit('timing', 'submit.retry', Date.now() - this.start, this); - this.submit(callback); + var request = this; + this.backend.trigger(this.backend.MIDDLEWARE_ACTIONS.retry, this.agent, this, function(err) { + if (err) return callback(err); + request.submit(callback); + }); }; // The fixups must be undone before the op is transformed forward again diff --git a/test/middleware.js b/test/middleware.js index 77cb80a5..d39e7e45 100644 --- a/test/middleware.js +++ b/test/middleware.js @@ -423,6 +423,126 @@ describe('middleware', function() { }); }); + describe('retry', function() { + var backend; + var doc; + var remoteDoc; + + beforeEach(function(done) { + backend = this.backend; + doc = backend.connect().get('dogs', 'fido'); + remoteDoc = backend.connect().get('dogs', 'fido'); + doc.create({age: 3}, function(error) { + if (error) return done(error); + remoteDoc.fetch(done); + }); + }); + + // Holds both ops in 'commit' until they've been applied to the same snapshot, + // then commits the remote op first, so the local op loses the race and retries + function forceRetry(callback) { + var commitDoc; + var commitRemoteDoc; + backend.use('commit', function(request, next) { + if (commitDoc && commitRemoteDoc) return next(); + if (request.op.src === doc.connection.id) commitDoc = next; + else commitRemoteDoc = next; + if (commitDoc && commitRemoteDoc) commitRemoteDoc(); + }); + + remoteDoc.submitOp([{p: ['age'], na: 7}], function(error) { + if (error) return callback(error); + commitDoc(); + }); + doc.submitOp([{p: ['age'], na: 2}], callback); + } + + it('is triggered between a lost commit and the next attempt', function(done) { + var actions = []; + ['submit', 'apply', 'commit', 'retry', 'afterWrite'].forEach(function(action) { + backend.use(action, function(request, next) { + if (request.op.src === doc.connection.id) actions.push(action); + next(); + }); + }); + + forceRetry(function(error) { + if (error) return done(error); + expect(actions).to.eql(['submit', 'apply', 'commit', 'retry', 'apply', 'commit', 'afterWrite']); + expect(doc.data).to.eql({age: 12}); + done(); + }); + }); + + it('has the number of retries so far', function(done) { + var retries = []; + backend.use('retry', function(request, next) { + retries.push(request.retries); + next(); + }); + + forceRetry(function(error) { + if (error) return done(error); + expect(retries).to.eql([1]); + done(); + }); + }); + + it('lets middleware reset state before the next attempt', function(done) { + var snapshotsBeforeApply = []; + backend.use('apply', function(request, next) { + if (request.op.src !== doc.connection.id) return next(); + if (!request.snapshotBeforeApply) { + request.snapshotBeforeApply = JSON.parse(JSON.stringify(request.snapshot.data)); + } + snapshotsBeforeApply.push(request.snapshotBeforeApply); + next(); + }); + backend.use('retry', function(request, next) { + delete request.snapshotBeforeApply; + next(); + }); + + forceRetry(function(error) { + if (error) return done(error); + expect(snapshotsBeforeApply).to.eql([{age: 3}, {age: 10}]); + done(); + }); + }); + + it('is not triggered if the op commits first time', function(done) { + var retry = sinon.spy(function(_request, next) { + next(); + }); + backend.use('retry', retry); + + doc.submitOp([{p: ['age'], na: 2}], function(error) { + if (error) return done(error); + expect(retry).not.to.have.been.called; + done(); + }); + }); + + it('is not triggered once the op has used up its retries', function(done) { + backend.maxSubmitRetries = 0; + var retry = sinon.spy(function(_request, next) { + next(); + }); + backend.use('retry', retry); + + forceRetry(function(error) { + expect(error.code).to.equal(ERROR_CODE.ERR_MAX_SUBMIT_RETRIES_EXCEEDED); + expect(retry).not.to.have.been.called; + done(); + }); + }); + + it('rejects the op with an error passed to next', function(done) { + backend.use('retry', passError); + forceRetry(getErrorTest(done)); + }); + }); + describe('$fixup', function() { var connection; var backend;