From ac48476fd1621f8841148effdb825a9217251d91 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 11 Sep 2019 14:09:45 +0800 Subject: [PATCH] crimson/net: discard future returned by close() It's OK to discard the returned future of Connection::close() because Messenger::shutdown() will wait for all connections closed. Signed-off-by: Yingxin Cheng --- src/crimson/net/Connection.h | 4 +++- src/crimson/net/ProtocolV1.cc | 18 +++++++----------- src/crimson/net/ProtocolV2.cc | 14 +++++++------- src/crimson/net/SocketConnection.cc | 5 ++--- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/crimson/net/Connection.h b/src/crimson/net/Connection.h index 8129c5351cf..9364df1c240 100644 --- a/src/crimson/net/Connection.h +++ b/src/crimson/net/Connection.h @@ -80,7 +80,9 @@ class Connection : public seastar::enable_shared_from_this { /// handshake virtual seastar::future<> keepalive() = 0; - /// close the connection and cancel any any pending futures from read/send + // close the connection and cancel any any pending futures from read/send + // Note it's OK to discard the returned future because Messenger::shutdown() + // will wait for all connections closed virtual seastar::future<> close() = 0; /// which shard id the connection lives diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc index 39418899799..c7c81679637 100644 --- a/src/crimson/net/ProtocolV1.cc +++ b/src/crimson/net/ProtocolV1.cc @@ -377,7 +377,7 @@ void ProtocolV1::start_connect(const entity_addr_t& _peer_addr, }).handle_exception([this] (std::exception_ptr eptr) { // TODO: handle fault in the connecting state logger().warn("{} connecting fault: {}", conn, eptr); - close(); + (void) close(); }); }); } @@ -466,11 +466,7 @@ seastar::future ProtocolV1::replace_existing( // will all be performed using v2 protocol. ceph_abort("lossless policy not supported for v1"); } - seastar::do_with( - std::move(existing), - [](auto existing) { - return existing->close(); - }); + (void) existing->close(); return send_connect_reply_ready(reply_tag, std::move(authorizer_reply)); } @@ -587,7 +583,7 @@ seastar::future ProtocolV1::repeat_handle_connect() logger().warn("{} existing {} proto version is {} not 1, close existing", conn, *existing, static_cast(existing->protocol->proto_type)); - existing->close(); + (void) existing->close(); } else { return handle_connect_with_existing(existing, std::move(authorizer_reply)); } @@ -667,7 +663,7 @@ void ProtocolV1::start_accept(SocketFRef&& sock, }).handle_exception([this] (std::exception_ptr eptr) { // TODO: handle fault in the accepting state logger().warn("{} accepting fault: {}", conn, eptr); - close(); + (void) close(); }); }); } @@ -906,13 +902,13 @@ void ProtocolV1::execute_open() return dispatcher.ms_handle_reset( seastar::static_pointer_cast(conn.shared_from_this())) .then([this] { - close(); + (void) close(); }); } else if (e.code() == error::read_eof) { return dispatcher.ms_handle_remote_reset( seastar::static_pointer_cast(conn.shared_from_this())) .then([this] { - close(); + (void) close(); }); } else { throw e; @@ -920,7 +916,7 @@ void ProtocolV1::execute_open() }).handle_exception([this] (std::exception_ptr eptr) { // TODO: handle fault in the open state logger().warn("{} open fault: {}", conn, eptr); - close(); + (void) close(); }); }); } diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc index 07fa0a4eab2..5022303ac29 100644 --- a/src/crimson/net/ProtocolV2.cc +++ b/src/crimson/net/ProtocolV2.cc @@ -67,7 +67,7 @@ void abort_protocol() { } void abort_in_close(ceph::net::ProtocolV2& proto) { - proto.close(); + (void) proto.close(); abort_protocol(); } @@ -444,7 +444,7 @@ void ProtocolV2::fault(bool backoff, const char* func_name, std::exception_ptr e logger().info("{} {}: fault at {} on lossy channel, going to CLOSING -- {}", conn, func_name, get_state_name(state), eptr); dispatch_reset(); - close(); + (void) close(); } else if (conn.policy.server || (conn.policy.standby && (!is_queued() && conn.sent.empty()))) { @@ -1201,7 +1201,7 @@ ProtocolV2::handle_existing_connection(SocketConnectionRef existing_conn) " existing connection {} is a lossy channel. Close existing in favor of" " this connection", conn, *existing_conn); existing_proto->dispatch_reset(); - existing_proto->close(); + (void) existing_proto->close(); if (unlikely(state != state_t::ACCEPTING)) { logger().debug("{} triggered {} in execute_accepting()", @@ -1337,7 +1337,7 @@ ProtocolV2::server_connect() conn, *existing_conn, static_cast(existing_conn->protocol->proto_type)); // should unregister the existing from msgr atomically - existing_conn->close(); + (void) existing_conn->close(); } else { return handle_existing_connection(existing_conn); } @@ -1446,7 +1446,7 @@ ProtocolV2::server_reconnect() "close existing and reset client.", conn, *existing_conn, static_cast(existing_conn->protocol->proto_type)); - existing_conn->close(); + (void) existing_conn->close(); return send_reset(true); } @@ -1600,7 +1600,7 @@ void ProtocolV2::execute_accepting() }).handle_exception([this] (std::exception_ptr eptr) { logger().info("{} execute_accepting(): fault at {}, going to CLOSING -- {}", conn, get_state_name(state), eptr); - close(); + (void) close(); }); }); } @@ -2137,7 +2137,7 @@ void ProtocolV2::execute_server_wait() }).handle_exception([this] (std::exception_ptr eptr) { logger().info("{} execute_server_wait(): fault at {}, going to CLOSING -- {}", conn, get_state_name(state), eptr); - close(); + (void) close(); }); }); } diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc index d3e4114ceea..dcf7ea41974 100644 --- a/src/crimson/net/SocketConnection.cc +++ b/src/crimson/net/SocketConnection.cc @@ -88,9 +88,8 @@ seastar::future<> SocketConnection::keepalive() seastar::future<> SocketConnection::close() { - return seastar::smp::submit_to(shard_id(), [this] { - return protocol->close(); - }); + ceph_assert(seastar::engine().cpu_id() == shard_id()); + return protocol->close(); } bool SocketConnection::update_rx_seq(seq_num_t seq) -- 2.39.5