Minor code improvements (#568)

CHANGELOG.md

@@ -5,7 +5,7 @@ Bob versions changelog
 ## [Unreleased]
 #### Added
 - Blob performes fsync if buffered bytes are larger than max_dirty_bytes_before_sync config param (#748)
-- Command to generate nodes section for  cluster.yaml or add new nodes to an existing cluster.yaml using range syntax patterns. (#568)
+- Command to generate nodes section for cluster.yaml or add new nodes to an existing cluster.yaml using range syntax patterns (#568)
 
 #### Changed
 - Use cargo workspace to declare dependencies to avoid their duplication (#821)

bob-apps/bin/ccg.rs

@@ -5,7 +5,6 @@ extern crate log;
 
 use anyhow::{anyhow, Result as AnyResult};
 use bob::ClusterConfig;
-use bob_common::configs::{cluster::DistributionFunc};
 use clap::{App, Arg, ArgMatches, SubCommand};
 use config_cluster_generator::{
     center::{check_expand_configs, get_new_disks, get_new_racks, Center},
@@ -209,7 +208,7 @@ fn simple_gen(
 
 fn pattern_gen(pattern: String, node_pattern: String) -> AnyResult<ClusterConfig> {
     let nodes = pattern_extend_nodes(vec![], pattern, node_pattern)?;
-    let config = ClusterConfig::new(nodes, vec![], vec![], DistributionFunc::default());
+    let config = ClusterConfig::new(nodes);
     debug!("pattern gen: OK [\n{:#?}\n]", config);
     Ok(config)
 }
@@ -220,7 +219,7 @@ fn pattern_expand(
     node_pattern: String,
 ) -> AnyResult<ClusterConfig> {
     let nodes = pattern_extend_nodes(config.nodes().to_owned(), pattern, node_pattern)?;
-    let config = ClusterConfig::new(nodes, config.vdisks().to_owned(), config.racks().to_owned(), config.distribution_func());
+    let config = ClusterConfig::new(nodes);
     debug!("pattern extending: OK [\n{:#?}\n]", config);
     Ok(config)
 }

bob-apps/bin/config_cluster_generator/pattern.rs

@@ -1,5 +1,6 @@
 use anyhow::{anyhow, Result as AnyResult};
 use bob_common::configs::cluster::Node;
+use bob_common::core_types::{DiskName, DiskPath};
 use itertools::Itertools;
 use regex::{Captures, Regex};
 
@@ -61,35 +62,47 @@ pub fn pattern_extend_nodes(
     let parsed_samples = generate_range_samples(&pattern)
         .map(|key| parse_address_sample(&key))
         .collect::<AnyResult<Vec<_>>>()?;
-
-    let mut node_counter = old_nodes.len();
-    for (ip_port, addresses) in parsed_samples
+    parsed_samples
         .iter()
         .group_by(|(ip_port, _)| ip_port)
         .into_iter()
-    {
-        let address = format!("{}:{}", ip_port.0, ip_port.1);
-        if let Some(node) = old_nodes.iter_mut().find(|node| node.address() == address) {
-            node.merge_disks(addresses.map(|(_, path)| path.to_owned()));
-        } else {
-            node_counter += 1;
-            let mut new_node = Node::new(
-                substitute_node_pattern(&node_pattern, &ip_port.0, ip_port.1, node_counter),
-                address,
-                vec![],
-            );
-            new_node.merge_disks(addresses.map(|(_, path)| path.to_owned()));
-            old_nodes.push(new_node);
-        }
-    }
-
+        .for_each(|(ip_port, addresses)| {
+            let address = format!("{}:{}", ip_port.0, ip_port.1);
+            let mut p_node = old_nodes.iter_mut().find(|node| node.address() == address);
+            if p_node.is_none() {
+                old_nodes.push(Node::new(
+                    substitute_node_pattern(
+                        &node_pattern,
+                        &ip_port.0,
+                        ip_port.1,
+                        old_nodes.len() + 1,
+                    ),
+                    address,
+                    vec![],
+                ));
+                p_node = old_nodes.last_mut();
+            }
+            let p_node = p_node.expect("is some because it's either the pushed one or found");
+            let old_disks = p_node.disks();
+            let new_disks: Vec<DiskPath> = addresses
+                .map(|(_, path)| path)
+                .filter(|disk_path| !p_node.disks().iter().any(|d| d.path() == *disk_path))
+                .enumerate()
+                .map(|(idx, disk_path)| {
+                    DiskPath::new(
+                        DiskName::new(&format!("disk{}", idx + old_disks.len() + 1)),
+                        disk_path.as_str(),
+                    )
+                })
+                .collect();
+            p_node.disks_extend(new_disks);
+        });
     Ok(old_nodes)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use bob_common::core_types::{DiskName, DiskPath};
 
     #[test]
     fn test_generate_range_samples() {

bob-common/src/configs/cluster.rs

@@ -105,18 +105,9 @@ impl Node {
         &self.disks
     }
 
-    /// Merges new disk paths into a node's disk list, without duplicates.
-    pub fn merge_disks(&mut self, new_disks_paths: impl Iterator<Item = String>) {
-        let mut disk_counter = self.disks().len() + 1;
-        for disk_path in new_disks_paths {
-            if !self.disks().iter().any(|d| d.path() == disk_path) {
-                self.disks.push(DiskPath::new(
-                    DiskName::new(&format!("disk{}", disk_counter)),
-                    disk_path.as_str(),
-                ));
-                disk_counter += 1;
-            }
-        }
+    /// Extends the disks collection with contents of the iterator.
+    pub fn disks_extend(&mut self, iter: impl IntoIterator<Item = DiskPath>) {
+        self.disks.extend(iter)
     }
 
     /// Returns node address, empty if address wasn't set in config.
@@ -271,15 +262,12 @@ pub struct Cluster {
 impl Cluster {
     pub fn new(
         nodes: Vec<Node>,
-        vdisks: Vec<VDisk>,
-        racks: Vec<Rack>,
-        distribution_func: DistributionFunc,
     ) -> Cluster {
         Cluster {
             nodes,
-            vdisks,
-            racks,
-            distribution_func,
+            vdisks: Vec::default(),
+            racks: Vec::default(),
+            distribution_func: DistributionFunc::default(),
         }
     }
     /// Returns slice with [`Node`]s.