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

rust: add demo Tonic server #4318

Merged
merged 11 commits into from
Nov 12, 2020
Merged

rust: add demo Tonic server #4318

merged 11 commits into from
Nov 12, 2020

Conversation

wchargin
Copy link
Contributor

Summary:
This patch defines a simple “demo” service with one RPC, which adds a
sequence of numbers. It includes a Tonic server for this service to
demonstrate the end-to-end setup.

Test Plan:
In one shell, run bazel run -c opt //tensorboard/data/server. In
another shell, use grpc_cli to send an RPC to localhost port 6789:

grpc_cli --channel_creds_type=insecure \
    --protofiles tensorboard/data/server/demo.proto \
    call localhost:6789 Demo.Add "term: 2 term: 2"

This should print a response like sum: 4. On my machine, it completes
in 5.2 ± 2.6 ms (methodology: run hyperfine on the above command).
This seems reasonably fast given that it has to establish a connection,
whereas a Python gRPC client will keep a connection open. It’s also well
below the 40ms magic number for TCP_NODELAY issues.

wchargin-branch: rust-demo-tonic-server

Summary:
This includes: `tonic`, a gRPC framework; `tonic-build`, its compiler
for protobuf bindings; `tokio`, an async runtime; and `async-stream`,
`async-trait`, and `futures-core`, helpers for streaming gRPC methods.

Test Plan:
All crates build without any manual configuration:

```
bazel build //third_party/rust:{tonic{,_build},tokio,async_{stream,trait},futures_core}
```

wchargin-branch: rust-dep-tonic-stack
wchargin-source: 0fa8ea005745e7194bde20da6fb1b9871c05822a
Summary:
This patch switches our Rust protocol buffer bindings from `prost-build`
to `tonic-build`. This generates the same protobuf message types, since
Tonic interoperates with Prost, but can now also generate gRPC stubs.

Test Plan:
Check the doc server to see that our existing proto types still look
good, in the `rustboard_core::proto::tensorboard` package. We don’t have
gRPC services to generate yet. A follow-up commit will add a demo server
as proof of concept.

wchargin-branch: rust-compile-grpc
wchargin-source: c5090247a8a25c1902d4b42597ec4d4ae2593779
Summary:
This patch defines a simple “demo” service with one RPC, which adds a
sequence of numbers. It includes a Tonic server for this service to
demonstrate the end-to-end setup.

Test Plan:
In one shell, run `bazel run -c opt //tensorboard/data/server`. In
another shell, use `grpc_cli` to send an RPC to localhost port 6789:

```
grpc_cli --channel_creds_type=insecure \
    --protofiles tensorboard/data/server/demo.proto \
    call localhost:6789 Demo.Add "term: 2 term: 2"
```

This should print a response like `sum: 4`. On my machine, it completes
in 5.2 ± 2.6 ms (methodology: run `hyperfine` on the above command).
This seems reasonably fast given that it has to establish a connection,
whereas a Python gRPC client will keep a connection open. It’s also well
below the 40ms magic number for `TCP_NODELAY` issues.

wchargin-branch: rust-demo-tonic-server
wchargin-source: 25d9fcbf2279742cdffde0c5357711244e9afef2
@wchargin wchargin added the core:rustboard //tensorboard/data/server/... label Nov 11, 2020
@google-cla google-cla bot added the cla: yes label Nov 11, 2020
wchargin-branch: rust-compile-grpc
wchargin-source: 6d888a758d42d318467189245ebcc619acaefe9c
wchargin-branch: rust-demo-tonic-server
wchargin-source: 2181a31c718433d5113cbb1fe2e51c7c98193cc1
wchargin-branch: rust-demo-tonic-server
wchargin-source: 2181a31c718433d5113cbb1fe2e51c7c98193cc1
wchargin-branch: rust-compile-grpc
wchargin-source: ba5e9a59b4b4f95e86b1f7a4a0b0d5c28c446476
wchargin-branch: rust-demo-tonic-server
wchargin-source: 89356565a9557266c069dcfdf0422f2e2225e3ab

# Conflicts:
#	tensorboard/data/server/gen_protos_tool.rs
wchargin-branch: rust-demo-tonic-server
wchargin-source: 89356565a9557266c069dcfdf0422f2e2225e3ab
Base automatically changed from wchargin-rust-compile-grpc to master November 12, 2020 05:50
wchargin-branch: rust-demo-tonic-server
wchargin-source: eb0bce9732ce4810d7152ec6eb47ba67a8d85d5e

# Conflicts:
#	tensorboard/data/server/gen_protos_tool.rs
wchargin-branch: rust-demo-tonic-server
wchargin-source: eb0bce9732ce4810d7152ec6eb47ba67a8d85d5e
@@ -28,6 +28,7 @@ fn main() -> std::io::Result<()> {
};
tonic_build::configure()
.out_dir(&out_dir)
.format(false) // don't run `rustfmt`; shouldn't be needed to build
Copy link
Contributor

Choose a reason for hiding this comment

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

ah :) I knew something was weird about the generated file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah—I had format(true) (the default) originally, but that only worked
on my machine because I have rustfmt installed; it failed on Travis.
It would be nice for it to be formatted. We could format it pretty
easily if bazelbuild/rules_rust#388 were resolved, or we could copy what
bazelbuild/rules_rust#479 does and do it the hard way. I figured that to
start we could just accept the ugly generated code: we have the Rustdoc
server for normal API usage questions, and you can always run rustfmt
on it locally if you want to look at the implementation.

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let addr = "[::0]:6789".parse::<std::net::SocketAddr>()?;
let handler = DemoHandler::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

One part that is iffy in my head is, how does it know to invoke async fn add for the Demo.Add service method?

Copy link
Contributor Author

@wchargin wchargin Nov 12, 2020

Choose a reason for hiding this comment

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

At a high level, the answer is: “the Demo trait, which we implemented
on our handler”. That trait contains one Rust method for each gRPC
method. The trait is defined in the code generated by tonic_build;
that generated code has the hard-coded mapping between gRPC method names
and Rust methods and types.

I’ve never read the generated code before, but we can take a look. It’s
got some stuff going on to handle gRPC machinery and async internals,
but we shouldn’t usually need to care about those. Here it is, lightly
annotated (from tensorboard.demo.pb.rs after rustfmt):

    impl<T, B> Service<http::Request<B>> for DemoServer<T>
    where
////////// Trait implemented for all handlers that implement the `Demo` trait,
////////// which includes our `DemoHandler` struct:
        T: Demo,
////////// The request and error types need to be moveable between threads,
////////// shareable across threads, and with only static borrowed data; this
////////// way, the server has more opportunities to use threads for handling:
        B: HttpBody + Send + Sync + 'static,
        B::Error: Into<StdError> + Send + 'static,
    {
        // ...
        fn call(&mut self, req: http::Request<B>) -> Self::Future {
            let inner = self.inner.clone();
            match req.uri().path() {
////////////////// Here's where we match against the gRPC request path...
                "/demo.Demo/Add" => {
                    #[allow(non_camel_case_types)]
                    struct AddSvc<T: Demo>(pub Arc<T>);
                    impl<T: Demo> tonic::server::UnaryService<super::AddRequest> for AddSvc<T> {
                        type Response = super::AddResponse;
                        type Future = BoxFuture<tonic::Response<Self::Response>, tonic::Status>;
                        fn call(
                            &mut self,
                            request: tonic::Request<super::AddRequest>,
                        ) -> Self::Future {
                            let inner = self.0.clone();
////////////////////////////// ...and here's where we call our `async fn add`:
                            let fut = async move { (*inner).add(request).await };
                            Box::pin(fut)
                        }
                    }
                    let inner = self.inner.clone();
                    let fut = async move {
                        let interceptor = inner.1.clone();
                        let inner = inner.0;
                        let method = AddSvc(inner);
                        let codec = tonic::codec::ProstCodec::default();
                        let mut grpc = if let Some(interceptor) = interceptor {
                            tonic::server::Grpc::with_interceptor(codec, interceptor)
                        } else {
                            tonic::server::Grpc::new(codec)
                        };
                        let res = grpc.unary(method, req).await;
                        Ok(res)
                    };
                    Box::pin(fut)
                }
////////////////// Here's the catch-all, with `Code.UNIMPLEMENTED == 12`:
                _ => Box::pin(async move {
                    Ok(http::Response::builder()
                        .status(200)
                        .header("grpc-status", "12")
                        .body(tonic::body::BoxBody::empty())
                        .unwrap())
                }),
            }
        }
    }

Server::builder()
.add_service(pb::demo_server::DemoServer::new(handler))
.serve(addr)
.await?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You didn’t comment on the postfix await syntax, so I will! :-)

When I saw that Rust was considering implementing await as a postfix
operator, like foo.await rather than await foo, I was pretty
skeptical, since it deviates from JS, Python, C#, etc. But then I came
across this example. Compare these two: which is easier to read?

// JavaScript: prefix `await` syntax
const data = (await (await fetch("data/runs")).json()).field;
// Rust: postfix `await` syntax
let data = fetch("data/runs").await?.json().await?.field;

Once I saw this, I was immediately sold. With prefix await, control
flow bounces back and forth:

const data = (await (await fetch("data/runs")).json()).field;  /*
                           ------------------ 1
                     ----- 2
                                               ------ 3
              ----- 4
                                                       ----- 5 */

and you have to deal with the associated parentheses: the first thing
that you see is await (await (...)). Getting the nesting right is
particularly annoying when manually sending fetches in the console.
With postfix await, it’s nice and linear, like a fluent method chain:

let data = fetch("data/runs").await?.json().await?.field;  /*
           --------------- 1  --- 2  --- 3  --- 4  --- 5   */

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let addr = "[::0]:6789".parse::<std::net::SocketAddr>()?;
let handler = DemoHandler::default();
Copy link
Contributor Author

@wchargin wchargin Nov 12, 2020

Choose a reason for hiding this comment

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

At a high level, the answer is: “the Demo trait, which we implemented
on our handler”. That trait contains one Rust method for each gRPC
method. The trait is defined in the code generated by tonic_build;
that generated code has the hard-coded mapping between gRPC method names
and Rust methods and types.

I’ve never read the generated code before, but we can take a look. It’s
got some stuff going on to handle gRPC machinery and async internals,
but we shouldn’t usually need to care about those. Here it is, lightly
annotated (from tensorboard.demo.pb.rs after rustfmt):

    impl<T, B> Service<http::Request<B>> for DemoServer<T>
    where
////////// Trait implemented for all handlers that implement the `Demo` trait,
////////// which includes our `DemoHandler` struct:
        T: Demo,
////////// The request and error types need to be moveable between threads,
////////// shareable across threads, and with only static borrowed data; this
////////// way, the server has more opportunities to use threads for handling:
        B: HttpBody + Send + Sync + 'static,
        B::Error: Into<StdError> + Send + 'static,
    {
        // ...
        fn call(&mut self, req: http::Request<B>) -> Self::Future {
            let inner = self.inner.clone();
            match req.uri().path() {
////////////////// Here's where we match against the gRPC request path...
                "/demo.Demo/Add" => {
                    #[allow(non_camel_case_types)]
                    struct AddSvc<T: Demo>(pub Arc<T>);
                    impl<T: Demo> tonic::server::UnaryService<super::AddRequest> for AddSvc<T> {
                        type Response = super::AddResponse;
                        type Future = BoxFuture<tonic::Response<Self::Response>, tonic::Status>;
                        fn call(
                            &mut self,
                            request: tonic::Request<super::AddRequest>,
                        ) -> Self::Future {
                            let inner = self.0.clone();
////////////////////////////// ...and here's where we call our `async fn add`:
                            let fut = async move { (*inner).add(request).await };
                            Box::pin(fut)
                        }
                    }
                    let inner = self.inner.clone();
                    let fut = async move {
                        let interceptor = inner.1.clone();
                        let inner = inner.0;
                        let method = AddSvc(inner);
                        let codec = tonic::codec::ProstCodec::default();
                        let mut grpc = if let Some(interceptor) = interceptor {
                            tonic::server::Grpc::with_interceptor(codec, interceptor)
                        } else {
                            tonic::server::Grpc::new(codec)
                        };
                        let res = grpc.unary(method, req).await;
                        Ok(res)
                    };
                    Box::pin(fut)
                }
////////////////// Here's the catch-all, with `Code.UNIMPLEMENTED == 12`:
                _ => Box::pin(async move {
                    Ok(http::Response::builder()
                        .status(200)
                        .header("grpc-status", "12")
                        .body(tonic::body::BoxBody::empty())
                        .unwrap())
                }),
            }
        }
    }

@@ -28,6 +28,7 @@ fn main() -> std::io::Result<()> {
};
tonic_build::configure()
.out_dir(&out_dir)
.format(false) // don't run `rustfmt`; shouldn't be needed to build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah—I had format(true) (the default) originally, but that only worked
on my machine because I have rustfmt installed; it failed on Travis.
It would be nice for it to be formatted. We could format it pretty
easily if bazelbuild/rules_rust#388 were resolved, or we could copy what
bazelbuild/rules_rust#479 does and do it the hard way. I figured that to
start we could just accept the ugly generated code: we have the Rustdoc
server for normal API usage questions, and you can always run rustfmt
on it locally if you want to look at the implementation.

@wchargin wchargin merged commit da59d4c into master Nov 12, 2020
@wchargin wchargin deleted the wchargin-rust-demo-tonic-server branch November 12, 2020 08:05
wchargin added a commit that referenced this pull request Nov 17, 2020
Summary:
When setting up the transitive dep on `libc` in #4316, I had originally
passed `--cfg=libc_align`, because that fixed the build for me and on
CI, and I couldn’t figure out the right cross-platform fix. But @faern
[helpfully points out that we just need to run the build script][1],
which detects the OS and Rust compiler version to determine the
appropriate flags. Bazel `rules_rust` can handle this, if we just ask.

(I should have followed my own `DEVELOPMENT.md` suggestions!)

[1]: rust-lang/socket2#130 (comment)

Test Plan:
It still builds—`bazel build @raze__libc__0_2_80//:libc`—and the demo
server still works and accepts requests following #4318’s test plan.

wchargin-branch: rust-libc-gen-buildrs
wchargin-source: 4ddff8d0b822585a9e84c7a24c4577b97b9c5d6a
wchargin added a commit that referenced this pull request Nov 17, 2020
Summary:
When setting up the transitive dep on `libc` in #4316, I had originally
passed `--cfg=libc_align`, because that fixed the build for me and on
CI, and I couldn’t figure out the right cross-platform fix. But @faern
[helpfully points out that we just need to run the build script][1],
which detects the OS and Rust compiler version to determine the
appropriate flags. Bazel `rules_rust` can handle this, if we just ask.

(I should have followed my own `DEVELOPMENT.md` suggestions!)

[1]: rust-lang/socket2#130 (comment)

Test Plan:
It still builds—`bazel build @raze__libc__0_2_80//:libc`—and the demo
server still works and accepts requests following #4318’s test plan.

wchargin-branch: rust-libc-gen-buildrs
wchargin added a commit that referenced this pull request Nov 19, 2020
Summary:
The `//tensorboard/data/server` binary now provides a gRPC server that
implements the `TensorBoardDataProvider` service. It only supports
scalars, and it has entirely fake data. But it’s enough to be wired up
to TensorBoard and show real charts in the UI.

Test Plan:
Run the server with `bazel run -c opt //tensorboard/data/server`.
(Running with `-c opt` is not necessary, but the server is notably
faster that way.) Then, in a different shell, use `grpc_cli`:

```
$ grpc_cli --channel_creds_type=insecure \
>   --protofiles tensorboard/data/proto/data_provider.proto \
>   call localhost:6806 TensorBoardDataProvider.ListRuns '' 2>/dev/null
runs {
  name: "train"
  start_time: 1605752017
}
```

Interestingly, this takes 13.9 ± 4.9 ms on my machine, whereas the demo
server added in #4318 took only 5.2 ± 2.6 ms. Both are basically doing
no work on the server, so I suspect that the difference may be due to
`grpc_cli` having to do more work to parse our real proto files. And,
indeed, making the calls from Python instead takes only 0.8–1.1 ms.

wchargin-branch: rust-real-fake-data-provider
wchargin-source: 67e69e8512a96443568ca916dd9559f4457ddf27
wchargin added a commit that referenced this pull request Nov 24, 2020
Summary:
The `//tensorboard/data/server` binary now provides a gRPC server that
implements the `TensorBoardDataProvider` service. It only supports
scalars, and it has entirely fake data. But it’s enough to be wired up
to TensorBoard and show real charts in the UI.

Test Plan:
Run the server with `bazel run -c opt //tensorboard/data/server`.
(Running with `-c opt` is not necessary, but the server is notably
faster that way.) Then, in a different shell, use `grpc_cli`:

```
$ grpc_cli --channel_creds_type=insecure \
>   --protofiles tensorboard/data/proto/data_provider.proto \
>   call localhost:6806 TensorBoardDataProvider.ListRuns '' 2>/dev/null
runs {
  name: "train"
  start_time: 1605752017
}
```

Interestingly, this takes 13.9 ± 4.9 ms on my machine, whereas the demo
server added in #4318 took only 5.2 ± 2.6 ms. Both are basically doing
no work on the server, so I suspect that the difference may be due to
`grpc_cli` having to do more work to parse our real proto files. And,
indeed, making the calls from Python instead takes only 0.8–1.1 ms.

wchargin-branch: rust-real-fake-data-provider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes core:rustboard //tensorboard/data/server/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants