Skip to content

Conversation

@w33ble
Copy link
Contributor

@w33ble w33ble commented Oct 25, 2018

Replaces #23301
Closes #23080


This is a minimal threading implementation for Canvas. There's still a lot to be done to make this concept great, but this is a start.

What it does:

  • Creates a server side abstraction on top of the interpreter
  • Determines where to send the expression by checking the first function to be run
  • Loads common functions in a separate worker thread on the server.
  • Routes to a single forked worker (thread), the main thread (server), or the browser (browser), in that order
  • Defers back to the router when a function isn't found. Fails if the function isn't found in any of the above 3 environments
  • Times out the worker if it takes too long, and respawns it as needed.
  • Simplifies the error dialog to remove the stack.

What is does not.:

  • Round robin a pool of workers
  • Queue. If one expression in the threaded env fails then anything sent to it in the meantime will fail. The upstream environment handles managing timeouts. I think this would only make sense todo with a pool.
  • Client side. This doesn't implement web workers, but we could use roughly the same architecture.
  • Implement a specific, pluggable worker environment on the server. Right now it's just common functions, so plugin authors will always end up in a thread if they put their function in the common directory.

What I don't like:

  • The socketProvider code. This was reused across the server & browser, but now that it's only used in the browser there's no good reason for the abstraction
  • The serialize/deserialize stuff feels messy. Do we really need serialization?

@w33ble w33ble added v7.0.0 v6.5.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v6.6.0 review labels Oct 25, 2018
@elastic elastic deleted a comment from elasticmachine Oct 25, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1 cqliu1 self-requested a review October 26, 2018 00:23
Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 works great!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@w33ble w33ble merged commit b8b0229 into elastic:master Oct 26, 2018
w33ble added a commit that referenced this pull request Oct 26, 2018
Replaces #23301
Closes #23080

---

This is a minimal threading implementation for Canvas. There's still a lot to be done to make this concept great, but this is a start. 

What it does:
- Creates a server side abstraction on top of the interpreter
- Determines where to send the expression by checking the first function to be run
- Loads common functions in a separate worker thread on the server. 
- Routes to a single forked worker (thread), the main thread (server), or the browser (browser), in that order
- Defers back to the router when a function isn't found. Fails if the function isn't found in any of the above 3 environments
- Times out the worker if it takes too long, and respawns it as needed.
- Simplifies the error dialog to remove the stack. 

What is does not.:
- Round robin a pool of workers
- Queue. If one expression in the threaded env fails then anything sent to it in the meantime will fail. The upstream environment handles managing timeouts. I think this would only make sense todo with a pool.
- Client side. This doesn't implement web workers, but we could use roughly the same architecture. 
- Implement a specific, pluggable `worker` environment on the server. Right now it's just common functions, so plugin authors will always end up in a thread if they put their function in the common directory.

What I don't like:
- The socketProvider code. This was reused across the server & browser, but now that it's only used in the browser there's no good reason for the abstraction
- The serialize/deserialize stuff feels messy. Do we really need serialization?
w33ble added a commit that referenced this pull request Oct 26, 2018
Replaces #23301
Closes #23080

---

This is a minimal threading implementation for Canvas. There's still a lot to be done to make this concept great, but this is a start. 

What it does:
- Creates a server side abstraction on top of the interpreter
- Determines where to send the expression by checking the first function to be run
- Loads common functions in a separate worker thread on the server. 
- Routes to a single forked worker (thread), the main thread (server), or the browser (browser), in that order
- Defers back to the router when a function isn't found. Fails if the function isn't found in any of the above 3 environments
- Times out the worker if it takes too long, and respawns it as needed.
- Simplifies the error dialog to remove the stack. 

What is does not.:
- Round robin a pool of workers
- Queue. If one expression in the threaded env fails then anything sent to it in the meantime will fail. The upstream environment handles managing timeouts. I think this would only make sense todo with a pool.
- Client side. This doesn't implement web workers, but we could use roughly the same architecture. 
- Implement a specific, pluggable `worker` environment on the server. Right now it's just common functions, so plugin authors will always end up in a thread if they put their function in the common directory.

What I don't like:
- The socketProvider code. This was reused across the server & browser, but now that it's only used in the browser there's no good reason for the abstraction
- The serialize/deserialize stuff feels messy. Do we really need serialization?
@w33ble
Copy link
Contributor Author

w33ble commented Oct 26, 2018

6.5 e8eef1e
6.x dc55efb

@w33ble w33ble deleted the feat/expression-threading branch October 26, 2018 19:41
XavierM added a commit that referenced this pull request Oct 29, 2018
* Translate global navigation bar component (#23993)

Translate global navigation bar component

* [backport] add back earlier 6.x minor versions

We still backport to these branches, primarily for doc changes.

* [dev/build] fix invalid assertion

* Skip this test until snapshots are updated (#24650)

* Feat/expression threading (#24598)

Replaces #23301
Closes #23080

---

This is a minimal threading implementation for Canvas. There's still a lot to be done to make this concept great, but this is a start. 

What it does:
- Creates a server side abstraction on top of the interpreter
- Determines where to send the expression by checking the first function to be run
- Loads common functions in a separate worker thread on the server. 
- Routes to a single forked worker (thread), the main thread (server), or the browser (browser), in that order
- Defers back to the router when a function isn't found. Fails if the function isn't found in any of the above 3 environments
- Times out the worker if it takes too long, and respawns it as needed.
- Simplifies the error dialog to remove the stack. 

What is does not.:
- Round robin a pool of workers
- Queue. If one expression in the threaded env fails then anything sent to it in the meantime will fail. The upstream environment handles managing timeouts. I think this would only make sense todo with a pool.
- Client side. This doesn't implement web workers, but we could use roughly the same architecture. 
- Implement a specific, pluggable `worker` environment on the server. Right now it's just common functions, so plugin authors will always end up in a thread if they put their function in the common directory.

What I don't like:
- The socketProvider code. This was reused across the server & browser, but now that it's only used in the browser there's no good reason for the abstraction
- The serialize/deserialize stuff feels messy. Do we really need serialization?

* Polish 6.5 (#24556)

* Updates waterfall item design for timeline rows

* Adjusts span and tx flyouts and updates tooltips to EUI

* Heading size fixes and clean up

* Updates tooltip snapshots

* Review tweaks and snapshot updates

* Revert experiment :)

Co-Authored-By: jasonrhodes <[email protected]>

* Fixes bug with v1 waterfall state

* Fixes bug with timeline bar height

* Updates snapshot tests

* Updated test so it doesn't mount and rely on EUI makeId() which is non-deterministic per test run

* Don't throw errors in optimizer (#24660)

* Fixed label position on progress elements (#24623)

* [kbn/es] add context to error message (#24664)

This just tweaks the kbn-es error message to provide more context than just `Not Found`

* [BeatsCM] Beats without tags should return an empty array via the config API (#24665)

* [ML] Change file data visualizer JSON format label to NDJSON (#24643)

* [ML] Change file datavisualizer JSON format label to NDJSON
* [ML] Update edit flyout overrides snapshot

* Translations for Coordinate Map (#23952)

translate Coordinate Map

* Translations for Region Map (#23875)

add translations for region_map plugin

* [Tools] Add TemplateLiteral parsing to i18n_check tool (#24580)

* [Tools] Add TemplateLiteral parsing to i18n_check tool

* Add comments

* [ML] Remove obsolete sentence from info tooltip. (#24716)

* Translate security/users component (#23940)

Translate security/users

* [Docs] Remove beta notes for ML and Query bar (#24718)

* Translations for Table Vis plugin (#23679)

add translations for table vis plugin

* Feature/translate new nav bar (#24326)

translate new_nav_bar

* center content in fullscreen mode, hide K7 top nav (#24589)

* [APM] Fixes rare cases where KibanaLink is loaded outside of React context (#24705)

* Fixes rare cases where KibanaLink will be loaded outside of React context and requires no redux connect dependency

* Fixes tests for updated Kibana link component

* Removes obsolete snapshot

* Secops structure code (#24652)

* add basic structure for secops application
* finalize skeleton for secops
* fix type issue and hapi new version
* remove route home, not needed for now
* Add configuration + delete noise
* prepend elastic license to generated file

* Cut down on all tests except for secops tests and one example of infr… (#24693)

* Cut down on all tests except for secops tests and one example of infra integration tests
* Commented out code for only this branch
* Added comments and "please see issue number"
* https://github.com/elastic/ingest-dev/issues/60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v6.5.0 v6.6.0 v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants