Implement Client/Server Editing #1043

sc1f · 2020-05-12T00:16:19Z

One of Perspective's main selling points is interoperability between perspective-viewer in the browser and perspective-python on the server. This PR furthers inter-op by enabling editing between a client and a server without the latency overhead of using a remote Table, i.e. proxying all operations to the server. By creating ports on the client and server, clients are able to make edits to a Perspective Table in browser memory, and automatically transfer their edits to the server and all other connected clients. Similarly, the server is able to propagate live updates to each of their connected clients, and state will remain automatically synchronized.

For a concise example, see the Python Examples folder, where the client_server_edits Python and HTML files show the simplest possible implementation of client/server editing. The benefits of using client/server editing as opposed to traditional remote table editing are numerous, including:

Lightning-fast performance on the client side, as the render is no longer tied to round-trips from the server.
All clients receive updates from the server as well as edits from all other clients.
Because the table the client sees is constructed within the client, it can be a subset/slice of the main dataset on the server.

The main tradeoff is loading time - remote tables load incredibly quickly, as there is no data to transport across the network besides what is being requested by the viewer. For a table to be constructed in the client-side, however, the client needs access to the entire table's dataset in Arrow form. The example provides a simple implementation of this, and for most datasets the tradeoff in loading time is far outweighed by increased performance.

Changelog

Add the make_port API to Table, and the getEditPort method to perspective-viewer, allowing clients and servers to create ports and enable client/server editing.
Changes the on_update API - depending on the language, parameters to on_update callback functions need to be changed in order for proper operation. issues with callback parameters are easily debuggable, as they will raise exceptions in both Javascript and Python.
- In Javascript, on_update callbacks now receive only one parameter: an object containing the keys port_id and delta, depending on if cell or row mode is selected:

view.on_update(updated => {
  console.log(updated.port_id, updated.delta instanceof ArrayBuffer);
}, {mode: "row"});

In Python, on_update callbacks either receive the port_id argument by default, or they receive both port_id and delta (if the row mode was selected):

def callback_with_delta(port_id, delta):
  print(port_id, delta)

def callback_without_delta(port_id):
  print(port_id)

view.on_update(callback_with_delta, mode="row")
view.on_update(callback_without_delta)

Change the way we process batched updates and notify users in on_update, which will always fire with a port_id.
Adds APIs on perspective-viewer so that each viewer automatically gets a port that is used for editing.
Added support for passing arrow binaries to and from PerspectiveWebsocket (in JS) and PerspectiveTornadohandler (in Python).
Fixes a regex mismatch that caused issues on Firefox but not Chrome.
Adds tests + examples in Javascript and Python

… not sure if this works for the model but we will find out

…_id)

texodus

This is a really, really cool feature, with awesome test coverage also!

Apologies for the long delay in reviewing and merging this. In my defense - I think a PR having its own Changelog is a pretty apt jump-the-shark moment in terms of monolithic PRs, even if only as this PR's exceptionally detailed description nor mini-Changelog will not show up in the lerna-changelog master CHANGELOG.md for releases. Which means you won't be able to reap those sweet, sweet internet points.

Totally worth the wait, this one PR is basically an entire release's worth of features in itself!

texodus · 2020-05-21T03:14:15Z

cpp/perspective/src/cpp/gnode.cpp

@@ -597,10 +646,16 @@ t_gnode::get_table_sptr() {
 void
 t_gnode::promote_column(const std::string& name, t_dtype new_type) {
    PSP_TRACE_SENTINEL();
-    PSP_VERBOSE_ASSERT(m_init, "touching uninited object");
+    PSP_VERBOSE_ASSERT(m_init, "Cannot `promote_column` on an uninited gnode.");


Much better error message! I assume you changed these to trace the segfault? Reminds me that a future improvement may be to completely replace PSP_VERBOSE_ASSERT which check object initialization only and replace with proper valgrind support in the build scripts, stricter clang warnings and our existing excellent test coverage?

texodus · 2020-05-21T03:16:24Z

cpp/perspective/src/cpp/gnode.cpp

+t_uindex
+t_gnode::num_output_ports() const {
+    return m_oports.size();
+}


I am OK with removing these too if you're keen.

texodus · 2020-05-21T03:20:33Z

packages/perspective-viewer/src/js/computed_expressions/lexer.js

@@ -32,7 +32,7 @@ const UpperLowerCaseTokenType = createToken({
 });

 // Create tokens for column names and computed function names
-const column_name_regex_pattern = /(["'])(?<column_name>.*?[^\\])\1/y;
+const column_name_regex_pattern = /(["'])(.*?[^\\])\1/y;


Chrome bug?

texodus · 2020-05-21T03:27:22Z

cpp/perspective/src/cpp/gnode.cpp

+
+    // Whether the user should be notified - False if process_table exited
+    // early, True otherwise.
+    return result.m_should_notify_userspace;


We could maybe even remove the concept of no-ops and drop m_should_notify_userspace altogether; instead, we can just say if _process_table made it past the empty-table-checks earlier in this method, we call notify_userspace. This may simplify some of the aggregate logic in the context_* classes as well since they won't need to report this information.

texodus · 2020-05-21T03:30:03Z

packages/perspective-bench/bench/versions.js

@@ -34,7 +34,7 @@ const JPMC_VERSIONS = [

 const FINOS_VERSIONS = ["0.3.1", "0.3.0", "0.3.0-rc.3", "0.3.0-rc.2", "0.3.0-rc.1"];

-const UMD_VERSIONS = ["0.4.5", "0.4.4", "0.4.2", "0.4.1", "0.4.0", "0.3.9", "0.3.8", "0.3.7", "0.3.6"];
+const UMD_VERSIONS = ["0.4.8", "0.4.7", "0.4.6", "0.4.5", "0.4.4", "0.4.2", "0.4.1", "0.4.0", "0.3.9", "0.3.8", "0.3.7", "0.3.6"];


Thanks for updating this - reminds me, we could just use this and dump the pre-finos versions I think now?

$ npm view @finos/perspective versions --json [ "0.3.0-rc.1", "0.3.0-rc.2", "0.3.0-rc.3", "0.3.0-rc.4", "0.3.0", "0.3.1", "0.3.4", "0.3.5", "0.3.6", "0.3.7", "0.3.8", "0.3.9", "0.4.0-rc.1", "0.4.0-rc.2", "0.4.0-rc.3", "0.4.0-rc.4", "0.4.0-rc.5", "0.4.0-rc.6", "0.4.0", "0.4.1", "0.4.2", "0.4.4", "0.4.5", "0.4.6", "0.4.7", "0.4.8" ]

texodus · 2020-05-21T03:33:01Z

packages/perspective-viewer/src/js/viewer.js

+     * @async
+     */
+    async getEditPort() {
+        return this._edit_port_lock;


This doesn't need to be async - _edit_port_lock is a Promise.

texodus · 2020-05-21T03:35:50Z

packages/perspective-viewer/src/js/viewer/perspective_element.js

+
+        // Resolve the edit port lock, which allows for `get_edit_port` to be
+        // called in arbitary order without ever returning a null value.
+        this._edit_port_lock.resolve(this._edit_port);


I think this is unnecessary if we remove the await on line 272 above? I would also like to understand how this can lead to null for port, since it seems (on inspection) that the Promise does not defer the actual call to make_port().

I tried removing this on my branch and did not see errors in the tests, but the message implies this error condition is race-y ..

texodus · 2020-05-21T03:36:30Z

packages/perspective/src/js/api/server.js

@@ -153,7 +153,8 @@ export class Server {
                        // post transferable data for arrow
                        if (msg.args && msg.args[0]) {
                            if (msg.method === "on_update" && msg.args[0]["mode"] === "row") {
-                                this.post(result, [ev]);
+                                // actual arrow is in the `delta`
+                                this.post(result, [ev.delta]);


Most Understated Addition in this patch!

texodus · 2020-05-21T03:51:04Z

Thanks for the PR!

finos-admin added the cla-present label May 12, 2020

sc1f requested review from timkpaine and texodus May 12, 2020 00:16

sc1f self-assigned this May 12, 2020

sc1f added C++ enhancement Feature requests or improvements internal Internal refactoring and code quality improvement JS Python labels May 12, 2020

sc1f added 17 commits May 11, 2020 21:27

iterate through all input ports

5fc55f0

WIP: pass in port_id, arbitary send to ports

3270f77

WIP: process by port id

957f95a

Make sure remote streaming works and that port_id are pushed to update

34ac441

Call update task once per port, after all ports have been processed -…

e4d00c4

… not sure if this works for the model but we will find out

Call notify per port, remove pool::flush and t_update_task::run(gnode…

d571652

…_id)

Store input ports in ordered map

2eb567b

Add and test dynamic ports

0678a1c

fix python build

a279584

Only notify ports with queued updates, add viewer API

73f6ead

fix issue with regex causing error in firefox

bc31b73

add more tests

21f81b9

add more tests for multi-table/server setups

554c8e8

allow updates over websocket with arrow, both in js and python

0723798

add simple client server editing example

1402232

make api names clearer - make_port, getEditPort

954dbd2

add websocket ping to client/server example

40dd19a

sc1f force-pushed the ports branch from 746f57f to 40dd19a Compare May 12, 2020 01:27

texodus reviewed May 21, 2020

View reviewed changes

texodus approved these changes May 21, 2020

View reviewed changes

texodus merged commit b2914ed into master May 21, 2020

texodus deleted the ports branch May 21, 2020 03:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Client/Server Editing #1043

Implement Client/Server Editing #1043

sc1f commented May 12, 2020 •

edited by texodus

Loading

texodus left a comment •

edited

Loading

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus May 21, 2020

texodus commented May 21, 2020

Implement Client/Server Editing #1043

Implement Client/Server Editing #1043

Conversation

sc1f commented May 12, 2020 • edited by texodus Loading

Changelog

texodus left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

texodus commented May 21, 2020

sc1f commented May 12, 2020 •

edited by texodus

Loading

texodus left a comment •

edited

Loading