Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPIC: Evaluate Jetty as replacement for Netty #1270

Closed
pete-petey opened this issue Sep 13, 2021 · 1 comment · Fixed by #1731
Closed

EPIC: Evaluate Jetty as replacement for Netty #1270

pete-petey opened this issue Sep 13, 2021 · 1 comment · Fixed by #1731
Assignees
Labels
epic Feature Epic (User Story) jetty
Milestone

Comments

@pete-petey
Copy link
Member

No description provided.

@pete-petey pete-petey added epic Feature Epic (User Story) jetty labels Sep 13, 2021
@pete-petey pete-petey added this to the Sept 2021 milestone Sep 13, 2021
@niloc132
Copy link
Member

niloc132 commented Sep 13, 2021

This issue is to track the idea of running Deephaven Core and its associated browser-based UI without requiring docker or a local envoy install. As netty in grpc-netty is hard to extend (to offer a built-in grpc-web proxy, a ssl-less websocket proxy, or a server for static html/js content), we're exploring an old branch to the grpc-java project which added support for the jetty webserver.

Whereas netty is being used an internal implementation to grpc-netty, the grpc-jetty project assumed that the grpc wiring could coexist in a standard jetty server, along-side other filters and servlets. This would allow us to

  • serve static content, such as the js api and the web UI
  • build or include a servlet filter, which is capable of rewriting grpc streams to grpc-web
  • build or include a servlet filter that can redirect websocket requests to grpc-web-enabled endpoints, in the same way as https://github.com/improbable-eng/grpc-web/ does in an external process

Each of these three removes the need for a single external process that we presently require - four distinct docker images are offered, along with a sample docker-compose.yml to show roughly how to wire them up, but they could as easily be run as local processes. Consolidating to a single process would still easily allow running from within docker, or behind an envoy proxy, but these would no longer be required, and setup/scaling could be simpler in some cases. Running natively on mac/windows or without the indirection of docker on linux would be possible too, making for easier debugging and integration with any other local resources.

Risks involved include any work needed to update the grpc-jetty project to latest grpc, and losing our hermetically-created python environment (we might want to produce jpy-dh wheels for other platforms and python versions).

Basic projected milestones of this epic:

  • "wake up" the async servlet branch of grpc-java, currently up to about 1.32.0 (latest is 1.40.1, we presently use 1.38.0). We should also either see about getting this accepted upstream, or using our own fork of io.grpc (there are some changes to the core project that we would need to get away from netty).
  • Implement a grpc-web frontend, either reimplementing how grpc is provided as a servlet (likely more efficient, but more code to copy/write), or putting a filter on the existing grpc servlet and translating on the fly (likely easier to implement, but might end up copying data more than necessary). Taking the proxy approach, we have a few reference implementations we can follow, in addition to the spec.
  • As above, implement a websocket frontend that either reimplements how grpc is provided as a servlet, or serves as a proxy for grpc-web or grpc web. This might come in two stages, first 1:1 websocket to stream, as https://github.com/improbable-eng/grpc-web/ does, and potentially later finding a way to reduce the number of websockets that need to be held open at a time for a client. In addition to the base implementation as a reference, we can also use the client transport differences
  • Select a strategy to make it easy for end users to select netty vs jetty, with or without docker, etc - we likely don't want to double the number of ways that are currently supported to start a server. This checkbox probably also includes shipping the static web resources.

niloc132 added a commit to niloc132/deephaven-core that referenced this issue Nov 23, 2021
This is a first step towards making the http server for grpc
replacable with Jetty.

Partial deephaven#1270
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Nov 23, 2021
This is a first step towards making the http server for grpc
replacable with Jetty.

Partial deephaven#1270
niloc132 added a commit that referenced this issue Nov 24, 2021
This is a first step towards making the http server for grpc
replacable with Jetty.

Partial #1270
niloc132 added a commit that referenced this issue Dec 17, 2021
This class is specific to application mode, and is even already in the package for app mode, and seems to have been included in the grpc-api server for DI purposes only, but need not be there (and split packages, etc).

Partial #1270
niloc132 added a commit that referenced this issue Dec 17, 2021
These classes are specific to uri but lived in grpc-api server packages. These could also end up in their own module just for reuse in other projects that want to register, expose their own set of resolvers, but the simplest refactor was to move to :java-client-uri.

Partial #1270
niloc132 added a commit that referenced this issue Dec 17, 2021
The "new" dependencies introduced in this commit were ones that were transitive before with no need to be (i.e. not part of the actual API, but exposed anyway). Some of these are negotiable, like what should the grpc-api project actually expose to downstream projects - this is just a proposal that appears to build as-is, cherry-picked from the jetty work, to make that eventual merge smaller.

Partial #1270
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 20, 2021
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 20, 2021
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 20, 2021
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 20, 2021
Dependencies are distributed to where they are used now, and
now-unnecessary proto/build.gradle is removed entirely.

Partial deephaven#1270
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 21, 2021
Dependencies are distributed to where they are used now, and
now-unnecessary proto/build.gradle is removed entirely.

Partial deephaven#1270
niloc132 added a commit that referenced this issue Dec 21, 2021
…#1719)

Dependencies are distributed to where they are used now, and
now-unnecessary proto/build.gradle is removed entirely.

Partial #1270
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Dec 21, 2021
See readme in this commit for details

Partial deephaven#1270
niloc132 added a commit that referenced this issue Dec 22, 2021
See readme in this commit for details

Partial #1270
niloc132 added a commit that referenced this issue Dec 28, 2021
This patch introduces a new server implementation that uses Jetty as the http server rather than Netty, so is able to avoid using nginx, grpc-proxy, and envoy to let browsers connect.

There are a few known bugs with this, and so for now this is not expected to replace the existing server which uses docker, until we've had more time to test and investigate.

In order to use Jetty, slf4j is updated to an alpha release.

Fixes #1270
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Feature Epic (User Story) jetty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants