grpc-benchmark: Worker and server implementation#2706
Conversation
arjan-bal
force-pushed
the
grpc-benchmark-server
branch
from
923b430 to
b9bce39
Compare
ejona86
left a comment
ejona86
left a comment
There was a problem hiding this comment.
Didn't get all the way through; sending what I have.
arjan-bal
force-pushed
the
grpc-benchmark-server
branch
2 times, most recently
from
38faf3f to
821047d
Compare
arjan-bal
force-pushed
the
grpc-benchmark-server
branch
from
821047d to
7af7498
Compare
| println!("Server creation requested."); | ||
|
|
||
| if benchmark_server.take().is_some() { | ||
| eprintln!("Server setup received when server already exists, shutting down the existing server"); |
There was a problem hiding this comment.
Does replacing the server actually work? I'd expect start() to fail because the port is already in use. I think we need to set benchmark_server = None here. And that might still be racy.
There was a problem hiding this comment.
I think if you abort the task in drop, instead of using the Notify, then it should be fine, but there might still be stuff going on at the OS level that prevents it from being reused immediately. I would expect that should be up to the driver to not do that, though, right?
|
|
||
| let mut server_builder = Server::builder(); | ||
| // Parse security config. | ||
| if let Some(securit_params) = config.security_params { |
| .serve_with_shutdown(addr, async { | ||
| rx.recv().await; | ||
| // Wait for the quit_worker response to be sent. | ||
| time::sleep(Duration::from_secs(1)).await; |
There was a problem hiding this comment.
Why do we sleep 1 second here? Will the client fail if the server hard shuts down before the call competes successfully or something?
|
|
||
| pub async fn run_worker(worker_port: u16) -> Result<(), Box<dyn std::error::Error>> { | ||
| let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), worker_port); | ||
| let (tx, mut rx) = mpsc::channel(1); |
| use nix::sys::resource::UsageWho; | ||
| use nix::sys::resource::getrusage; | ||
| use nix::sys::time::TimeValLike; |
There was a problem hiding this comment.
What's the philosophy for this? When do we use at a local scope like this vs. at the file level? It would probably be good to have some kind of guideline if it's not a hard rule like "always use at the file level".
| println!("Server creation requested."); | ||
|
|
||
| if benchmark_server.take().is_some() { | ||
| eprintln!("Server setup received when server already exists, shutting down the existing server"); |
There was a problem hiding this comment.
I think if you abort the task in drop, instead of using the Notify, then it should be fine, but there might still be stuff going on at the OS level that prevents it from being reused immediately. I would expect that should be up to the driver to not do that, though, right?
3 participants
Contributes to: #2666
This PR adds the worker and the benchmarking server implementations. Both components use
tonicandprostfor now, as the gRPC server is still under development. The benchmarking client that uses gRPC will be added in a follow-up PR.