Skip to content

Commit e8ec9c5

Browse files
committed
Change: EffectiveMembership::get_node() should return an Option
`EffectiveMembership::get_node()` should return an `Option<&Node>` instead of a `&Node`. Otherwise it panic if the node is not found.
1 parent 61d47bc commit e8ec9c5

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

openraft/src/core/raft_core.rs

+14-4
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ impl<C: RaftTypeConfig, N: RaftNetworkFactory<C>, S: RaftStorage<C>> RaftCore<C,
326326
};
327327

328328
let my_id = self.id;
329-
let target_node = self.engine.state.membership_state.effective.get_node(&target).clone();
329+
// Safe unwrap(): target is in membership
330+
let target_node = self.engine.state.membership_state.effective.get_node(&target).unwrap().clone();
330331
let mut client = match self.network.new_client(target, &target_node).await {
331332
Ok(n) => n,
332333
Err(e) => {
@@ -891,7 +892,12 @@ impl<C: RaftTypeConfig, N: RaftNetworkFactory<C>, S: RaftStorage<C>> RaftCore<C,
891892
}
892893

893894
pub(crate) fn get_leader_node(&self, leader_id: Option<C::NodeId>) -> Option<C::Node> {
894-
leader_id.map(|id| self.engine.state.membership_state.effective.get_node(&id).clone())
895+
let leader_id = match leader_id {
896+
None => return None,
897+
Some(x) => x,
898+
};
899+
900+
self.engine.state.membership_state.effective.get_node(&leader_id).cloned()
895901
}
896902

897903
#[tracing::instrument(level = "debug", skip_all)]
@@ -979,7 +985,9 @@ impl<C: RaftTypeConfig, N: RaftNetworkFactory<C>, S: RaftStorage<C>> RaftCore<C,
979985
target: C::NodeId,
980986
matched: Option<LogId<C::NodeId>>,
981987
) -> Result<ReplicationStream<C::NodeId>, N::ConnectionError> {
982-
let target_node = self.engine.state.membership_state.effective.get_node(&target);
988+
// Safe unwrap(): target must be in membership
989+
let target_node = self.engine.state.membership_state.effective.get_node(&target).unwrap();
990+
983991
let membership_log_id = self.engine.state.membership_state.effective.log_id;
984992
let network = self.network.new_client(target, target_node).await?;
985993

@@ -1125,7 +1133,9 @@ impl<C: RaftTypeConfig, N: RaftNetworkFactory<C>, S: RaftStorage<C>> RaftCore<C,
11251133
}
11261134

11271135
let req = vote_req.clone();
1128-
let target_node = self.engine.state.membership_state.effective.get_node(&target).clone();
1136+
1137+
// Safe unwrap(): target must be in membership
1138+
let target_node = self.engine.state.membership_state.effective.get_node(&target).unwrap().clone();
11291139
let mut client = match self.network.new_client(target, &target_node).await {
11301140
Ok(n) => n,
11311141
Err(err) => {

openraft/src/membership/effective_membership.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ where
151151
}
152152

153153
/// Get a the node(either voter or learner) by node id.
154-
pub fn get_node(&self, node_id: &NID) -> &N {
154+
pub fn get_node(&self, node_id: &NID) -> Option<&N> {
155155
self.membership.get_node(node_id)
156156
}
157157

openraft/src/membership/membership.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use core::option::Option;
21
use std::collections::BTreeMap;
32
use std::collections::BTreeSet;
43

@@ -117,7 +116,7 @@ where
117116
}
118117
res.push(format!("{}", node_id));
119118

120-
let n = self.get_node(node_id);
119+
let n = self.get_node(node_id).unwrap();
121120
res.push(format!(":{{{:?}}}", n));
122121
}
123122
res.push("}".to_string());
@@ -135,7 +134,7 @@ where
135134

136135
res.push(format!("{}", learner_id));
137136

138-
let n = self.get_node(learner_id);
137+
let n = self.get_node(learner_id).unwrap();
139138
res.push(format!(":{{{:?}}}", n));
140139
}
141140
res.push("]".to_string());
@@ -262,8 +261,8 @@ where
262261
}
263262

264263
/// Get a the node(either voter or learner) by node id.
265-
pub(crate) fn get_node(&self, node_id: &NID) -> &N {
266-
&self.nodes[node_id]
264+
pub(crate) fn get_node(&self, node_id: &NID) -> Option<&N> {
265+
self.nodes.get(node_id)
267266
}
268267

269268
/// Returns an Iterator of all nodes(voters and learners).

0 commit comments

Comments
 (0)