Slightly simplify code at places

This commit is contained in:
Alex 2022-11-08 16:15:45 +01:00
parent 217abdca18
commit ec12d6c8dd
Signed by untrusted user: lx
GPG key ID: 0E496D15096376BE
2 changed files with 22 additions and 50 deletions

View file

@ -209,16 +209,13 @@ pub async fn cmd_show_layout(
"You can also revert all proposed changes with: garage layout revert --version {}", "You can also revert all proposed changes with: garage layout revert --version {}",
v + 1) v + 1)
} }
Err(Error::Message(s)) => { Err(e) => {
println!("Error while trying to compute the assignation: {}", s); println!("Error while trying to compute the assignation: {}", e);
println!("This new layout cannot yet be applied."); println!("This new layout cannot yet be applied.");
println!( println!(
"You can also revert all proposed changes with: garage layout revert --version {}", "You can also revert all proposed changes with: garage layout revert --version {}",
v + 1) v + 1)
} }
_ => {
println!("Unknown Error");
}
} }
} }
@ -355,7 +352,7 @@ pub fn print_cluster_layout(layout: &ClusterLayout) -> bool {
id, id,
tags, tags,
role.zone, role.zone,
role.capacity_string(), role.capacity_string()
)); ));
}; };
} }
@ -372,7 +369,7 @@ pub fn print_cluster_layout(layout: &ClusterLayout) -> bool {
} }
pub fn print_staging_parameters_changes(layout: &ClusterLayout) -> bool { pub fn print_staging_parameters_changes(layout: &ClusterLayout) -> bool {
let has_changes = layout.staging_parameters.get().clone() != layout.parameters; let has_changes = *layout.staging_parameters.get() != layout.parameters;
if has_changes { if has_changes {
println!(); println!();
println!("==== NEW LAYOUT PARAMETERS ===="); println!("==== NEW LAYOUT PARAMETERS ====");

View file

@ -100,16 +100,7 @@ impl NodeRole {
} }
pub fn tags_string(&self) -> String { pub fn tags_string(&self) -> String {
let mut tags = String::new(); self.tags.join(",")
if self.tags.is_empty() {
return tags;
}
tags.push_str(&self.tags[0].clone());
for t in 1..self.tags.len() {
tags.push(',');
tags.push_str(&self.tags[t].clone());
}
tags
} }
} }
@ -241,7 +232,7 @@ To know the correct value of the new layout version, invoke `garage layout show`
} }
/// Returns the uuids of the non_gateway nodes in self.node_id_vec. /// Returns the uuids of the non_gateway nodes in self.node_id_vec.
pub fn nongateway_nodes(&self) -> Vec<Uuid> { fn nongateway_nodes(&self) -> Vec<Uuid> {
let mut result = Vec::<Uuid>::new(); let mut result = Vec::<Uuid>::new();
for uuid in self.node_id_vec.iter() { for uuid in self.node_id_vec.iter() {
match self.node_role(uuid) { match self.node_role(uuid) {
@ -253,7 +244,7 @@ To know the correct value of the new layout version, invoke `garage layout show`
} }
/// Given a node uuids, this function returns the label of its zone /// Given a node uuids, this function returns the label of its zone
pub fn get_node_zone(&self, uuid: &Uuid) -> Result<String, Error> { fn get_node_zone(&self, uuid: &Uuid) -> Result<String, Error> {
match self.node_role(uuid) { match self.node_role(uuid) {
Some(role) => Ok(role.zone.clone()), Some(role) => Ok(role.zone.clone()),
_ => Err(Error::Message( _ => Err(Error::Message(
@ -299,7 +290,7 @@ To know the correct value of the new layout version, invoke `garage layout show`
} }
/// Returns the sum of capacities of non gateway nodes in the cluster /// Returns the sum of capacities of non gateway nodes in the cluster
pub fn get_total_capacity(&self) -> Result<u64, Error> { fn get_total_capacity(&self) -> Result<u64, Error> {
let mut total_capacity = 0; let mut total_capacity = 0;
for uuid in self.nongateway_nodes().iter() { for uuid in self.nongateway_nodes().iter() {
total_capacity += self.get_node_capacity(uuid)?; total_capacity += self.get_node_capacity(uuid)?;
@ -494,8 +485,7 @@ impl ClusterLayout {
if partition_size < 100 { if partition_size < 100 {
msg.push( msg.push(
"WARNING: The partition size is low (< 100), you might consider to \ "WARNING: The partition size is low (< 100), make sure the capacities of your nodes are correct and are of at least a few MB"
provide the nodes capacities in a smaller unit (e.g. Mb instead of Gb)."
.into(), .into(),
); );
} }
@ -533,7 +523,7 @@ impl ClusterLayout {
// (1) We compute the new node list // (1) We compute the new node list
// Non gateway nodes should be coded on 8bits, hence they must be first in the list // Non gateway nodes should be coded on 8bits, hence they must be first in the list
// We build the new node ids // We build the new node ids
let mut new_non_gateway_nodes: Vec<Uuid> = self let new_non_gateway_nodes: Vec<Uuid> = self
.roles .roles
.items() .items()
.iter() .iter()
@ -549,7 +539,7 @@ impl ClusterLayout {
))); )));
} }
let mut new_gateway_nodes: Vec<Uuid> = self let new_gateway_nodes: Vec<Uuid> = self
.roles .roles
.items() .items()
.iter() .iter()
@ -558,8 +548,8 @@ impl ClusterLayout {
.collect(); .collect();
let mut new_node_id_vec = Vec::<Uuid>::new(); let mut new_node_id_vec = Vec::<Uuid>::new();
new_node_id_vec.append(&mut new_non_gateway_nodes); new_node_id_vec.extend(new_non_gateway_nodes);
new_node_id_vec.append(&mut new_gateway_nodes); new_node_id_vec.extend(new_gateway_nodes);
let old_node_id_vec = self.node_id_vec.clone(); let old_node_id_vec = self.node_id_vec.clone();
self.node_id_vec = new_node_id_vec.clone(); self.node_id_vec = new_node_id_vec.clone();
@ -567,12 +557,11 @@ impl ClusterLayout {
// (2) We retrieve the old association // (2) We retrieve the old association
// We rewrite the old association with the new indices. We only consider partition // We rewrite the old association with the new indices. We only consider partition
// to node assignations where the node is still in use. // to node assignations where the node is still in use.
let mut old_assignation = vec![Vec::<usize>::new(); NB_PARTITIONS];
if self.ring_assignation_data.is_empty() { if self.ring_assignation_data.is_empty() {
// This is a new association // This is a new association
return Ok(None); return Ok(None);
} }
if self.ring_assignation_data.len() != NB_PARTITIONS * self.replication_factor { if self.ring_assignation_data.len() != NB_PARTITIONS * self.replication_factor {
return Err(Error::Message( return Err(Error::Message(
"The old assignation does not have a size corresponding to \ "The old assignation does not have a size corresponding to \
@ -590,7 +579,9 @@ impl ClusterLayout {
uuid_to_new_id.insert(*uuid, i); uuid_to_new_id.insert(*uuid, i);
} }
let mut old_assignation = vec![Vec::<usize>::new(); NB_PARTITIONS];
let rf = self.replication_factor; let rf = self.replication_factor;
for (p, old_assign_p) in old_assignation.iter_mut().enumerate() { for (p, old_assign_p) in old_assignation.iter_mut().enumerate() {
for old_id in &self.ring_assignation_data[p * rf..(p + 1) * rf] { for old_id in &self.ring_assignation_data[p * rf..(p + 1) * rf] {
let uuid = old_node_id_vec[*old_id as usize]; let uuid = old_node_id_vec[*old_id as usize];
@ -613,20 +604,12 @@ impl ClusterLayout {
let mut zone_to_id = HashMap::<String, usize>::new(); let mut zone_to_id = HashMap::<String, usize>::new();
for uuid in self.nongateway_nodes().iter() { for uuid in self.nongateway_nodes().iter() {
if self.roles.get(uuid) == None { let r = self.node_role(uuid).unwrap();
return Err(Error::Message(
"The uuid was not found in the node roles (this should \
not happen, it might be a critical error)."
.into(),
));
}
if let Some(r) = self.node_role(uuid) {
if !zone_to_id.contains_key(&r.zone) && r.capacity != None { if !zone_to_id.contains_key(&r.zone) && r.capacity != None {
zone_to_id.insert(r.zone.clone(), id_to_zone.len()); zone_to_id.insert(r.zone.clone(), id_to_zone.len());
id_to_zone.push(r.zone.clone()); id_to_zone.push(r.zone.clone());
} }
} }
}
Ok((id_to_zone, zone_to_id)) Ok((id_to_zone, zone_to_id))
} }
@ -639,11 +622,7 @@ impl ClusterLayout {
let empty_set = HashSet::<(usize, usize)>::new(); let empty_set = HashSet::<(usize, usize)>::new();
let mut g = self.generate_flow_graph(1, zone_to_id, &empty_set)?; let mut g = self.generate_flow_graph(1, zone_to_id, &empty_set)?;
g.compute_maximal_flow()?; g.compute_maximal_flow()?;
if g.get_flow_value()? if g.get_flow_value()? < (NB_PARTITIONS * self.replication_factor) as i64 {
< (NB_PARTITIONS * self.replication_factor)
.try_into()
.unwrap()
{
return Err(Error::Message( return Err(Error::Message(
"The storage capacity of he cluster is to small. It is \ "The storage capacity of he cluster is to small. It is \
impossible to store partitions of size 1." impossible to store partitions of size 1."
@ -656,11 +635,7 @@ impl ClusterLayout {
while s_down + 1 < s_up { while s_down + 1 < s_up {
g = self.generate_flow_graph((s_down + s_up) / 2, zone_to_id, &empty_set)?; g = self.generate_flow_graph((s_down + s_up) / 2, zone_to_id, &empty_set)?;
g.compute_maximal_flow()?; g.compute_maximal_flow()?;
if g.get_flow_value()? if g.get_flow_value()? < (NB_PARTITIONS * self.replication_factor) as i64 {
< (NB_PARTITIONS * self.replication_factor)
.try_into()
.unwrap()
{
s_up = (s_down + s_up) / 2; s_up = (s_down + s_up) / 2;
} else { } else {
s_down = (s_down + s_up) / 2; s_down = (s_down + s_up) / 2;