From 103c0238d4f39b9dd95a735142445d30e27c0b22 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sun, 9 Dec 2018 11:44:36 -0600 Subject: [PATCH] msg/async: preserve peer features when replacing a connection The features are now stored in the protocol implementation. When we replace an existing connection, copy those features so that our connect_msg_reply calculates the correct features for the session. This fixes an issue where a 3-mon cluster, after upgrading the two followers but not the leader, was unable to include the (luminous) leader in the quorum because it was seeing missing features in the connect reply, because the new mons were replacing an old instance of the connection and weren't copying the features, and that old instance had connect_msg.features == 0. Add some debug lines that helped (finally) identify the problem. Signed-off-by: Sage Weil --- src/msg/async/ProtocolV1.cc | 12 ++++++++++++ src/msg/async/ProtocolV2.cc | 2 ++ 2 files changed, 14 insertions(+) diff --git a/src/msg/async/ProtocolV1.cc b/src/msg/async/ProtocolV1.cc index ca3a801e49163..e21d8ed1f6c73 100644 --- a/src/msg/async/ProtocolV1.cc +++ b/src/msg/async/ProtocolV1.cc @@ -1862,6 +1862,8 @@ CtPtr ProtocolV1::handle_connect_message_2() { << " policy.server=" << connection->policy.server << " policy.standby=" << connection->policy.standby << " policy.resetcheck=" << connection->policy.resetcheck + << " features 0x" << std::hex << (uint64_t)connect_msg.features + << std::dec << dendl; ceph_msg_connect_reply reply; @@ -2144,6 +2146,14 @@ CtPtr ProtocolV1::send_connect_message_reply(char tag, reply.authorizer_len = authorizer_reply.length(); reply_bl.append((char *)&reply, sizeof(reply)); + ldout(cct, 10) << __func__ << " reply features 0x" << std::hex + << reply.features << " = (policy sup 0x" + << connection->policy.features_supported + << " & connect 0x" << (uint64_t)connect_msg.features + << ") | policy req 0x" + << connection->policy.features_required + << dendl; + if (reply.authorizer_len) { reply_bl.append(authorizer_reply.c_str(), authorizer_reply.length()); authorizer_reply.clear(); @@ -2197,6 +2207,8 @@ CtPtr ProtocolV1::replace(AsyncConnectionRef existing, } exproto->reset_recv_state(); + exproto->connect_msg.features = connect_msg.features; + auto temp_cs = std::move(connection->cs); EventCenter *new_center = connection->center; Worker *new_worker = connection->worker; diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index aefbefb86e00f..9bbe10aed2dcd 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -2239,6 +2239,8 @@ CtPtr ProtocolV2::replace(AsyncConnectionRef existing, } exproto->reset_recv_state(); + exproto->connect_msg.features = connect_msg.features; + auto temp_cs = std::move(connection->cs); EventCenter *new_center = connection->center; Worker *new_worker = connection->worker; -- 2.39.5