Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/middleware/actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the thinking of not actively resetting the snapshot just that we get more information this way about the thing that's about to retry? I slightly wonder if this is a foot gun waiting to go off, but I guess so long as it's documented.


This action has the same additional `context` properties as [`'submit'`](#submit).
Comment thread
dawidreedsy marked this conversation as resolved.

## `'afterWrite'`

An operation and its updated snapshot were successfully written to the database.
Expand Down
25 changes: 24 additions & 1 deletion docs/middleware/op-submission.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand All @@ -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()
})
Comment thread
dawidreedsy marked this conversation as resolved.
```

### 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.
Expand Down
2 changes: 2 additions & 0 deletions lib/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
dawidreedsy marked this conversation as resolved.
retry: 'retry',
Comment thread
dawidreedsy marked this conversation as resolved.
// An operation is about to be submitted to the database
submit: 'submit'
};
Expand Down
6 changes: 5 additions & 1 deletion lib/submit-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
dawidreedsy marked this conversation as resolved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth documenting the fact that this hook happens after resetting op state from fixups. I think that's the correct thing to do (since an op after fixups is kind of meaningless at this point), but it might feel inconsistent against the snapshot, which hasn't been reset. Or maybe it's a hint that we should unset the snapshot...?

if (err) return callback(err);
request.submit(callback);
});
};

// The fixups must be undone before the op is transformed forward again
Expand Down
120 changes: 120 additions & 0 deletions test/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
dawidreedsy marked this conversation as resolved.
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;
Expand Down
Loading