Skip to content

Commit

Permalink
[Sharded-Execution-GRPC] Address some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
manudhundi committed Oct 3, 2023
1 parent 84e0088 commit 610363e
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 4 deletions.
1 change: 0 additions & 1 deletion execution/executor-service/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ pub fn create_thread_remote_executor_shards(
}

#[test]
#[ignore]
fn test_sharded_block_executor_no_conflict() {
use std::thread;

Expand Down
9 changes: 6 additions & 3 deletions secure/net/src/grpc_network_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ impl GRPCNetworkMessageServiceServerWrapper {
.unwrap();

info!("Starting Server async at {:?}", server_addr);
// NOTE: serve_with_shutdown() starts the server, if successful the task does not return
// till the server is shutdown. Hence this should be called as a separate non-blocking task.
// Signal handler 'server_shutdown_rx' is needed to shutdown the server
// NOTE: (1) serve_with_shutdown() starts the server, if successful the task does not return
// till the server is shutdown. Hence this should be called as a separate
// non-blocking task. Signal handler 'server_shutdown_rx' is needed to shutdown
// the server
// (2) There is no easy way to know if/when the server has started successfully. Hence
// we may need to implement a healthcheck service to check if the server is up
Server::builder()
.timeout(std::time::Duration::from_millis(rpc_timeout_ms))
.add_service(NetworkMessageServiceServer::new(self))
Expand Down
3 changes: 3 additions & 0 deletions secure/net/src/network_controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ impl NetworkController {
self.outbound_task_shutdown_tx = self.outbound_handler.start(&self.outbound_rpc_runtime);
}

// TODO: This is still not a very clean shutdown. We don't wait for the full shutdown after
// sending the signal. May not matter much for now because we shutdown before exiting the
// process. Ideally, we want to fix this.
pub fn shutdown(&mut self) {
info!("Shutting down network controller at {}", self.listen_addr);
if let Some(shutdown_signal) = self.inbound_server_shutdown_tx.take() {
Expand Down

0 comments on commit 610363e

Please sign in to comment.