Deuxfleurs/garage
32
61
Fork 80
Code Issues 91 Pull requests 14 Releases 31 Wiki Activity

fix crash in layout computation when changing all nodes of a zone to gateway mode #937

Merged
lx merged 1 commit from baptiste/garage:fix_layout_crash into main 2025-02-19 17:09:11 +00:00
Conversation 1 Commits 1 Files changed 1 +10 -4
1 changed files with 10 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
src/rpc/layout/version.rs
View file

@ -650,8 +650,11 @@ impl LayoutVersion {
let mut cost = CostFunction::new(); let mut cost = CostFunction::new();
for (p, assoc_p) in prev_assign.iter().enumerate() { for (p, assoc_p) in prev_assign.iter().enumerate() {
for n in assoc_p.iter() { for n in assoc_p.iter() {
lx commented 2025-02-14 12:35:28 +00:00
Review
Copy link

Maybe add a // FIXME comment to indicate that we are not sure that this is the correct solution.

Even if this is the "wrong" solution, I don't think it will have a big impact. At worse, too much data will be migrated between nodes, but I don't think it can crash Garage or put it in an invalid state.

Maybe add a `// FIXME` comment to indicate that we are not sure that this is the correct solution. Even if this is the "wrong" solution, I don't think it will have a big impact. At worse, too much data will be migrated between nodes, but I don't think it can crash Garage or put it in an invalid state.
let node_zone = zone_to_id[self.expect_get_node_zone(&self.node_id_vec[*n])]; if let Some(&node_zone) =
cost.insert((Vertex::PZ(p, node_zone), Vertex::N(*n)), -1); zone_to_id.get(self.expect_get_node_zone(&self.node_id_vec[*n]))
{
cost.insert((Vertex::PZ(p, node_zone), Vertex::N(*n)), -1);
}
} }
} }
@ -751,8 +754,11 @@ impl LayoutVersion {
if let Some(prev_assign) = prev_assign_opt { if let Some(prev_assign) = prev_assign_opt {
let mut old_zones_of_p = Vec::<usize>::new(); let mut old_zones_of_p = Vec::<usize>::new();
for n in prev_assign[p].iter() { for n in prev_assign[p].iter() {
old_zones_of_p if let Some(&zone_id) =
.push(zone_to_id[self.expect_get_node_zone(&self.node_id_vec[*n])]); zone_to_id.get(self.expect_get_node_zone(&self.node_id_vec[*n]))
lx commented 2025-02-14 12:36:22 +00:00
Review
Copy link

This one does not impact correctness of the layout computation, at worse wrong statistics will be shown but I think that's ok to leave for a future issue if it happens.

This one does not impact correctness of the layout computation, at worse wrong statistics will be shown but I think that's ok to leave for a future issue if it happens.
{
old_zones_of_p.push(zone_id);
}
} }
if !old_zones_of_p.contains(&z) { if !old_zones_of_p.contains(&z) {
new_partitions_zone[z] += 1; new_partitions_zone[z] += 1;