[774] BobClient reduce clone overhead

CHANGELOG.md

@@ -7,6 +7,9 @@ Bob versions changelog
 - Added mimalloc allocator for musl target (#688)
 
 #### Changed
+- BobClient clone overhead reduced (#774)
+- Node struct internals placed inside Arc to reduce clone overhead (#724)
+- NodeName and DiskName types introduced to reduce clone overhead (#775)
 - Avoid Pearl Storage clone (#791)
 - Make iouring optional (#567)
 

bob-apps/bin/bobd.rs

@@ -36,13 +36,19 @@ async fn main() {
         return;
     }
 
-    let cluster_config = matches.value_of("cluster").unwrap();
+    let cluster_config = matches.value_of("cluster").expect("'cluster' argument is required");
     println!("Cluster config: {:?}", cluster_config);
-    let cluster = ClusterConfig::try_get(cluster_config).await.unwrap();
+    let cluster = ClusterConfig::try_get(cluster_config).await.map_err(|err| {
+        eprintln!("Cluster config parsing error: {}", err);
+        err
+    }).expect("Cluster config parsing error");
 
-    let node_config_file = matches.value_of("node").unwrap();
+    let node_config_file = matches.value_of("node").expect("'node' argument is required");
     println!("Node config: {:?}", node_config_file);
-    let node = cluster.get(node_config_file).await.unwrap();
+    let node = cluster.get(node_config_file).await.map_err(|err| {
+        eprintln!("Node config parsing error: {}", err);
+        err
+    }).expect("Node config parsing error");
 
     let mut extra_logstash_fields = HashMap::new();
     extra_logstash_fields.insert("node_name".to_string(), serde_json::Value::String(node.name().to_string()));

bob-apps/bin/config_cluster_generator/center.rs

@@ -11,8 +11,8 @@ use std::{
 
 const REPLICA_IN_FIRST_RACK: usize = 2;
 type RackName = String;
-type NodeName = String;
-type DiskName = String;
+type NodeName = bob_common::node::NodeName;
+type DiskName = bob_common::core_types::DiskName;
 
 #[derive(Debug, Default)]
 struct Counter {
@@ -126,7 +126,7 @@ impl Center {
                     },
                 )?;
             for node in rack.nodes() {
-                node_rack_map.insert(node, rack.name())
+                node_rack_map.insert(NodeName::from(node), rack.name())
             .map_or_else(
                     || Ok(()),
                 |_| {
@@ -238,7 +238,7 @@ impl Center {
         self.iter_disks()
             .find(|(_, node, disk)| {
                 self.counter.disk_used_count(&node.name, &disk.name) == min_key
-                    && list.contains(&Replica::new(node.name.clone(), disk.name.clone()))
+                    && list.contains(&Replica::new(node.name.to_string(), disk.name.to_string()))
             })
             .ok_or_else(|| anyhow!("Can't find disk"))
     }
@@ -445,7 +445,7 @@ impl Rack {
         let (node, disk) = disks
             .find(|(node, disk)| {
                 min_key == counter.disk_used_count(&node.name, &disk.name)
-                    && list.contains(&Replica::new(node.name.clone(), disk.name.clone()))
+                    && list.contains(&Replica::new(node.name.to_string(), disk.name.to_string()))
             })
             .ok_or_else(|| anyhow!("Can't find disk"))?;
 
@@ -521,7 +521,7 @@ impl Disk {
 }
 
 fn get_used_nodes_names(replicas: &[Replica]) -> Vec<NodeName> {
-    replicas.iter().map(|r| r.node().to_string()).collect()
+    replicas.iter().map(|r| NodeName::from(r.node())).collect()
 }
 
 pub fn get_new_disks(
@@ -541,7 +541,7 @@ pub fn get_new_disks(
                     && disk == &old_disk
             })
         })
-        .map(|(node, disk)| (node.name().to_owned(), disk.name().to_owned()))
+        .map(|(node, disk)| (NodeName::from(node.name()), DiskName::from(disk.name())))
         .collect()
 }
 

bob-backend/src/core.rs

@@ -21,16 +21,16 @@ pub const BACKEND_STARTED: f64 = 1f64;
 pub struct Operation {
     vdisk_id: VDiskId,
     disk_path: Option<DiskPath>,
-    remote_node_name: Option<String>, // save data to alien/<remote_node_name>
+    remote_node_name: Option<NodeName>, // save data to alien/<remote_node_name>
 }
 
 impl Operation {
     pub fn vdisk_id(&self) -> VDiskId {
         self.vdisk_id
     }
 
-    pub fn remote_node_name(&self) -> Option<&str> {
-        self.remote_node_name.as_deref()
+    pub fn remote_node_name(&self) -> Option<&NodeName> {
+        self.remote_node_name.as_ref()
     }
 }
 
@@ -62,18 +62,8 @@ impl Operation {
         }
     }
 
-    // local operation doesn't contain remote node, so node name is passed through argument
-    #[must_use]
-    pub fn clone_local_alien(&self, local_node_name: &str) -> Self {
-        Self {
-            vdisk_id: self.vdisk_id,
-            disk_path: None,
-            remote_node_name: Some(local_node_name.to_owned()),
-        }
-    }
-
     #[inline]
-    pub fn set_remote_folder(&mut self, name: String) {
+    pub fn set_remote_node_name(&mut self, name: NodeName) {
         self.remote_node_name = Some(name);
     }
 
@@ -83,8 +73,8 @@ impl Operation {
     }
 
     #[inline]
-    pub fn disk_name_local(&self) -> String {
-        self.disk_path.as_ref().expect("no path").name().to_owned()
+    pub fn disk_name_local(&self) -> &DiskName {
+        self.disk_path.as_ref().expect("no path").name()
     }
 }
 
@@ -163,7 +153,7 @@ pub trait MetricsProducer: Send + Sync {
 
 #[derive(Hash, PartialEq, Eq, Debug)]
 enum BackendErrorAction {
-    PUT(String, String),
+    PUT(DiskName, String),
 }
 
 impl Display for BackendErrorAction {
@@ -181,7 +171,7 @@ impl Display for BackendErrorAction {
 }
 
 impl BackendErrorAction {
-    fn put(disk: String, error: &Error) -> Self {
+    fn put(disk: DiskName, error: &Error) -> Self {
         BackendErrorAction::PUT(disk, error.to_string())
     }
 }
@@ -263,7 +253,7 @@ impl Backend {
             for node_name in options.remote_nodes() {
                 debug!("PUT[{}] core backend put remote node: {}", key, node_name);
                 let mut op = Operation::new_alien(vdisk_id);
-                op.set_remote_folder(node_name.to_owned());
+                op.set_remote_node_name(node_name.clone());
 
                 //TODO make it parallel?
                 self.put_single(key, data, op).await?;
@@ -327,12 +317,12 @@ impl Backend {
                         local_err
                     );
                     let error_to_log =
-                        BackendErrorAction::put(operation.disk_name_local(), &local_err);
+                        BackendErrorAction::put(operation.disk_name_local().clone(), &local_err);
                     self.error_logger.report_error(error_to_log);
 
                     // write to alien/<local name>
-                    let mut op = operation.clone_local_alien(self.mapper().local_node_name());
-                    op.set_remote_folder(self.mapper.local_node_name().to_owned());
+                    let mut op = operation.clone();
+                    op.set_remote_node_name(self.mapper.local_node_name().clone());
                     self.inner
                         .put_alien(op, key, data)
                         .await
@@ -454,7 +444,7 @@ impl Backend {
             for node in self.mapper.get_target_nodes_for_key(key) {
                 let force_delete = options.is_force_delete(node.name());
                 let mut op = Operation::new_alien(vdisk_id);
-                op.set_remote_folder(node.name().to_owned());
+                op.set_remote_node_name(node.name().clone());
                 let delete_res = self.delete_single(key, meta, op, force_delete).await;
                 if let Err(err) = delete_res {
                     error!("DELETE[{}] Error deleting from aliens (node: {}, force_delete: {}): {:?}", key, node.name(), force_delete, err);

bob-backend/src/lib.rs

@@ -22,7 +22,10 @@ pub(crate) mod prelude {
             cluster::Cluster as ClusterConfig,
             node::{BackendType, Node as NodeConfig, Pearl as PearlConfig},
         },
-        data::{BobData, BobKey, BobMeta, BobPutOptions, BobGetOptions, BobDeleteOptions, DiskPath, VDiskId},
+        data::{BobData, BobKey, BobMeta},
+        operation_options::{BobPutOptions, BobGetOptions, BobDeleteOptions},
+        core_types::{DiskName, DiskPath, VDiskId},
+        node::NodeName,
         error::Error,
         mapper::Virtual,
         metrics::BACKEND_STATE,

bob-backend/src/mem_backend.rs

@@ -46,17 +46,17 @@ impl VDisk {
 
 #[derive(Clone, Debug)]
 pub struct MemDisk {
-    pub name: String,
+    pub name: DiskName,
     pub vdisks: HashMap<VDiskId, VDisk>,
 }
 
 impl MemDisk {
-    pub fn new_direct(name: String, vdisks_count: u32) -> Self {
+    pub fn new_direct(name: DiskName, vdisks_count: u32) -> Self {
         let vdisks = (0..vdisks_count).map(|i| (i, VDisk::default())).collect();
         Self { name, vdisks }
     }
 
-    pub fn new(name: String, mapper: &Virtual) -> Self {
+    pub fn new(name: DiskName, mapper: &Virtual) -> Self {
         let vdisks = mapper
             .get_vdisks_by_disk(&name)
             .iter()
@@ -109,7 +109,7 @@ impl MemDisk {
 
 #[derive(Clone, Debug)]
 pub struct MemBackend {
-    pub disks: HashMap<String, MemDisk>,
+    pub disks: HashMap<DiskName, MemDisk>,
     pub foreign_data: MemDisk,
 }
 
@@ -119,11 +119,11 @@ impl MemBackend {
             .local_disks()
             .iter()
             .map(DiskPath::name)
-            .map(|name| (name.to_string(), MemDisk::new(name.to_string(), mapper)))
+            .map(|name| (name.clone(), MemDisk::new(name.clone(), mapper)))
             .collect();
         Self {
             disks,
-            foreign_data: MemDisk::new_direct("foreign".to_string(), mapper.vdisks_count()),
+            foreign_data: MemDisk::new_direct("foreign".into(), mapper.vdisks_count()),
         }
     }
 }

bob-backend/src/mem_tests.rs

@@ -10,11 +10,11 @@ const VDISKS_COUNT: u32 = 10;
 pub(crate) fn new_direct(paths: &[String], vdisks_count: u32) -> MemBackend {
     let disks = paths
         .iter()
-        .map(|p| (p.clone(), MemDisk::new_direct(p.clone(), vdisks_count)))
+        .map(|p| (DiskName::from(p), MemDisk::new_direct(DiskName::from(p), vdisks_count)))
         .collect();
     MemBackend {
         disks,
-        foreign_data: MemDisk::new_direct("foreign".to_string(), vdisks_count),
+        foreign_data: MemDisk::new_direct(DiskName::from("foreign"), vdisks_count),
     }
 }
 
@@ -24,7 +24,7 @@ async fn test_mem_put_wrong_disk() {
 
     let retval = backend
         .put(
-            Operation::new_local(0, DiskPath::new("invalid name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("invalid name".into(), "")),
             BobKey::from(1u64),
             &BobData::new(vec![0].into(), BobMeta::stub()),
         )
@@ -38,15 +38,15 @@ async fn test_mem_put_get() {
 
     backend
         .put(
-            Operation::new_local(0, DiskPath::new("name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("name".into(), "")),
             BobKey::from(1u64),
             &BobData::new(vec![1].into(), BobMeta::stub()),
         )
         .await
         .unwrap();
     let retval = backend
         .get(
-            Operation::new_local(0, DiskPath::new("name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("name".into(), "")),
             BobKey::from(1u64),
         )
         .await
@@ -60,15 +60,15 @@ async fn test_mem_get_wrong_disk() {
 
     backend
         .put(
-            Operation::new_local(0, DiskPath::new("name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("name".into(), "")),
             BobKey::from(1u64),
             &BobData::new(vec![1].into(), BobMeta::stub()),
         )
         .await
         .unwrap();
     let retval = backend
         .get(
-            Operation::new_local(0, DiskPath::new("invalid name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("invalid name".into(), "")),
             BobKey::from(1u64),
         )
         .await;
@@ -82,7 +82,7 @@ async fn test_mem_get_no_data() {
 
     let retval = backend
         .get(
-            Operation::new_local(0, DiskPath::new("name".to_owned(), "".to_owned())),
+            Operation::new_local(0, DiskPath::new("name".into(), "")),
             key,
         )
         .await;

bob-backend/src/pearl/disk_controller.rs

@@ -36,7 +36,7 @@ pub struct DiskController {
     vdisks: Vec<VDiskId>,
     run_sem: Arc<Semaphore>,
     monitor_sem: Arc<Semaphore>,
-    node_name: String,
+    node_name: NodeName,
     groups: Arc<RwLock<Vec<Group>>>,
     state: Arc<RwLock<GroupsState>>,
     settings: Arc<Settings>,
@@ -66,7 +66,7 @@ impl DiskController {
             vdisks,
             run_sem,
             monitor_sem: Arc::new(Semaphore::new(1)),
-            node_name: config.name().to_owned(),
+            node_name: NodeName::from(config.name()),
             groups: Arc::new(RwLock::new(Vec::new())),
             state: Arc::new(RwLock::new(GroupsState::NotReady)),
             settings,
@@ -260,13 +260,14 @@ impl DiskController {
             .copied()
             .map(|vdisk_id| {
                 let path = self.settings.normal_path(self.disk.path(), vdisk_id);
+                let owner_node_identifier = self.node_name.to_string();
                 Group::new(
                     self.settings.clone(),
                     vdisk_id,
                     self.node_name.clone(),
-                    self.disk.name().to_owned(),
+                    self.disk.name().clone(),
                     path,
-                    self.node_name.clone(),
+                    owner_node_identifier,
                     self.pearl_creation_context.clone(),
                 )
             })
@@ -277,7 +278,7 @@ impl DiskController {
         let settings = self.settings.clone();
         let groups = settings
             .collect_alien_groups(
-                self.disk.name().to_owned(),
+                self.disk.name(),
                 &self.node_name,
                 self.pearl_creation_context.clone(),
             )
@@ -325,7 +326,7 @@ impl DiskController {
             .settings
             .clone()
             .create_alien_group(
-                operation.remote_node_name().expect("Node name not found"),
+                operation.remote_node_name().expect("Node name not found").clone(),
                 operation.vdisk_id(),
                 &self.node_name,
                 self.pearl_creation_context.clone(),

bob-backend/src/pearl/disk_controller/logger.rs

@@ -27,7 +27,7 @@ impl DisksEventsLogger {
         f.sync_all().await
     }
 
-    pub(crate) async fn log(&self, disk_name: &str, event: &str, is_alien: bool) {
+    pub(crate) async fn log(&self, disk_name: &DiskName, event: &str, is_alien: bool) {
         let cur_time = Local::now();
         let log_msg = format!(
             "{};{};{};{}\n",