Held ops #384

alecgibson · 2020-07-23T15:06:37Z

Introduction

This change adds support for a "held op". Held ops are ops which are:

Only applied locally
Not necessarily submitted to the server
May be submtted to the server at some later time
Will be lost at the end of a "session" if not flushed (since they
haven't been submitted to the server)

Motivation

Held ops allow for some level of disconnect with the server, whilst
keeping the Doc "live". The primary use case this was designed for is
for when a Doc is in a "confirming" state: that is, some change has
been made to the Doc, but we're waiting for some confirmation that
this change should be comitted to the server (eg user input).

This sort of thing can be achieved with pause() and resume(), but:

Pausing a document will stop all ops from being submitted, even if
those other ops don't need confirmation
Resuming a Doc is not fool-proof. It would be a much nicer failure
mode to only fail to commit the changes that require confirmation,
rather than all subsequent changes

We could try to address these issues by having a side-channel: a second
Connection which we funnel all ops that need confirmation, which we
can safely pause, but this has its own issues:

Can't flush individual ops (unless we keep opening more
side-channels)
We now have two clients to handle, which can cause confusion around
how to merge data, and what constitutes a "local" op (ie everything
submitted down the pauseable connection, will look remote to the
mainline channel, even though it came from our own machine)

Held ops attempts to solve this issue by providing a mechanism for
submitting everything through a single client, and holding ops that can
be individually flushed to the server.

Note that one other possible (ab)use for held ops is to simply have
local-only data, which benefits from type transformations, although it
should be possible to achieve something like that with Presence.

API

We add a new method for submitting a held op:

var flush = doc.submitHeldOp(op, options);

Where op and options are the same as for submitOp. We could
potentially add a flag to options, like {held: true}, and just use
submitOp, but submitHeldOp returns a value, which submitOp does
not.

The returned value flush is a function that takes a callback:

flush(function(error) {})

This method will flush that particular held op to the server, and then
call the provided callback on acknowledgement, as if we'd used
submitOp.

# Introduction This change adds support for a "held op". Held ops are ops which are: - Only applied locally - Not necessarily submitted to the server - May be submtted to the server at some later time - Will be lost at the end of a "session" if not flushed (since they haven't been submitted to the server) # Motivation Held ops allow for some level of disconnect with the server, whilst keeping the `Doc` "live". The primary use case this was designed for is for when a `Doc` is in a "confirming" state: that is, some change has been made to the `Doc`, but we're waiting for some confirmation that this change should be comitted to the server (eg user input). This sort of thing can be achieved with `pause()` and `resume()`, but: - Pausing a document will stop all ops from being submitted, even if those other ops don't need confirmation - Resuming a `Doc` is not fool-proof. It would be a much nicer failure mode to only fail to commit the changes that require confirmation, rather than all subsequent changes We could try to address these issues by having a side-channel: a second `Connection` which we funnel all ops that need confirmation, which we can safely pause, but this has its own issues: - Can't flush individual ops (unless we keep opening more side-channels) - We now have two clients to handle, which can cause confusion around how to merge data, and what constitutes a "local" op (ie everything submitted down the pauseable connection, will look remote to the mainline channel, even though it came from our own machine) Held ops attempts to solve this issue by providing a mechanism for submitting everything through a single client, and holding ops that can be individually flushed to the server. Note that one other possible (ab)use for held ops is to simply have local-only data, which benefits from type transformations, although it should be possible to achieve something like that with Presence. # API We add a new method for submitting a held op: ```js var flush = doc.submitHeldOp(op, options); ``` Where `op` and `options` are the same as for `submitOp`. We could potentially add a flag to `options`, like `{held: true}`, and just use `submitOp`, but `submitHeldOp` returns a value, which `submitOp` does not. The returned value `flush` is a function that takes a callback: ```js flush(function(error) {}) ``` This method will flush that particular held op to the server, and then call the provided callback on acknowledgement, as if we'd used `submitOp`.

coveralls · 2020-07-23T15:19:00Z

Coverage increased (+0.004%) to 97.361% when pulling 9e4f385 on held-ops into 5218767 on master.

alecgibson · 2020-07-23T15:16:23Z

lib/client/doc.js

@@ -88,6 +88,9 @@ function Doc(connection, collection, id) {
  // This is a list of {[create:{...}], [del:true], [op:...], callbacks:[...]}
  this.pendingOps = [];

+  this._heldOpId = 1;
+  this._heldOps = {};


I know that the other props on this class aren't prefixed, but I'd like to keep things "private" where possible

alecgibson · 2020-07-23T15:18:45Z

lib/client/doc.js

+  var opsToTransform = [];
+  if (this.inflightOp) opsToTransform.push(this.inflightOp);
+  opsToTransform = opsToTransform.concat(this.pendingOps);
+  for (var id in this._heldOps) {


This is where my understanding of OT starts to get pushed a bit — does the transformation ordering matter here? I'd have normally assumed no, but I think transformX mutates the server op, so order does matter (I confirmed this trivially by moving the inflight op to after pending, and a couple of tests broke). I just don't know how (because held ops are temporally separate from pending ops). It could be that it's only important for create and del?

alecgibson · 2020-07-23T15:19:36Z

lib/client/doc.js

@@ -741,6 +737,19 @@ Doc.prototype._submit = function(op, source, callback) {
  });
 };

+// Try to normalize the op. This removes trailing skip:0's and things like that.
+Doc.prototype._normalizeOp = function(op) {
+  if (!this.type) {


This isn't really "normalising", but it's common code. Open to naming improvements on the method.

alecgibson · 2020-07-23T15:20:29Z

lib/client/doc.js

+
+  if (op.del) this._heldOps = {};
+  for (var id in this._heldOps) {
+    var transformErr = transformX(op, this._heldOps[id]);


Again, my understanding of transformX isn't great. It's going to mutate op. But we're at the end of the function, so I assume that's okay?

alecgibson · 2020-08-05T17:02:15Z

test/client/doc.js

+      });
+    });
+
+    it('agrees with a remote client about a conflicting local insert', function(done) {


Should also add a test case where the submit op has an index higher than the held op. This currently fails, because the submitted op should be transformed against held ops when being submitted, but they aren't.

ericyhwang

Rough summary of discussion:

An incoming server op probably don't care about the exact order that it gets transformed against the pending/held ops, as long as it gets them all eventually. If local ops have P1 - H1 - P2, then this PR's current _handleOp would transform by P1 - P2 - H1, which should mean the server op ends up at the same place.

One thing to test - If the client types "Hello" as a held op, types " world" as a normally submitted op, then flushes the held "Hello", does the final server doc state end up as "Hello world"?

alecgibson · 2020-10-16T10:47:35Z

I've tried looking at this again, but I think I've hit a snag where we need the type to be invertible. Consider this case:

Start with document {text: 'Hi!'}
Submit held op: [{p: ['text', 0], sd: 'Hi'}, {p: ['text', 0], si: 'Hello}]
Submit (non-held) op: [{p: ['text', 5], si: ' world'}]
Local document reads Hello world!
Remote document reads Hi! world (or potentially errors, given that 5 is longer than the string length?)

This is because we'd like the client to really send [{p: ['text', 2], si: ' world'}] — that is, the op as if the held op hadn't been applied.

However, as far as I'm aware, this is only achievable by transforming our submitted op by the inverse of the held op (or another way of thinking it might be to "untransform" by the op).

Only supporting invertible types is a definite shame (and on a personal note, wouldn't be acceptable for our own use case).

The only solution I can think of is pretty horrible: force consumers to call submitOp as if they're submitting to the remote doc (ie they ignore their own held ops), and then we can correctly transform the submitted op when applying locally. I don't know if I'm missing something better and cleverer?

curran · 2020-10-21T16:11:15Z

This is cool. Related to vizhub-core/vizhub-feedback#445

My take: If it only works on invertible types, that's OK as long as we document it as a caveat.

alecgibson · 2020-10-21T16:42:26Z

As discussed in today's meeting, I'm going to close this for now. I don't have a lot of reason to work on it if it doesn't support non-invertible types, and it means we'd probably have code lurking that nobody's using.

If anyone's particularly keen to add this feature, please feel free to pick up the work!

alecgibson commented Jul 23, 2020

View reviewed changes

alecgibson requested a review from ericyhwang July 23, 2020 15:20

alecgibson commented Aug 5, 2020

View reviewed changes

ericyhwang reviewed Aug 5, 2020

View reviewed changes

Transform sent ops by inverted held ops

9e4f385

curran mentioned this pull request Oct 21, 2020

Allow editing of an unforked viz vizhub-core/vizhub-feedback#445

Open

alecgibson closed this Oct 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Held ops #384

Held ops #384

alecgibson commented Jul 23, 2020

coveralls commented Jul 23, 2020 •

edited

alecgibson Jul 23, 2020

alecgibson Jul 23, 2020

alecgibson Jul 23, 2020

alecgibson Jul 23, 2020

alecgibson Aug 5, 2020

ericyhwang left a comment

alecgibson commented Oct 16, 2020

curran commented Oct 21, 2020 •

edited

alecgibson commented Oct 21, 2020

Held ops #384

Held ops #384

Conversation

alecgibson commented Jul 23, 2020

Introduction

Motivation

API

coveralls commented Jul 23, 2020 • edited

alecgibson Jul 23, 2020

Choose a reason for hiding this comment

alecgibson Jul 23, 2020

Choose a reason for hiding this comment

alecgibson Jul 23, 2020

Choose a reason for hiding this comment

alecgibson Jul 23, 2020

Choose a reason for hiding this comment

alecgibson Aug 5, 2020

Choose a reason for hiding this comment

ericyhwang left a comment

Choose a reason for hiding this comment

alecgibson commented Oct 16, 2020

curran commented Oct 21, 2020 • edited

alecgibson commented Oct 21, 2020

coveralls commented Jul 23, 2020 •

edited

curran commented Oct 21, 2020 •

edited