Using Arc<str> for the NodeId. (#186)

Co-authored-by: Paul Masurel <paul.masurel@datadoghq.com>

Author: fulmicoton

LICENSE-3rdparty.csv

@@ -7,13 +7,14 @@ bytes,https://github.com/tokio-rs/bytes,MIT,"Carl Lerche <me@carllerche.com>, Se
 cfg-if,https://github.com/rust-lang/cfg-if,MIT OR Apache-2.0,Alex Crichton <alex@alexcrichton.com>
 chacha20,https://github.com/RustCrypto/stream-ciphers,MIT OR Apache-2.0,RustCrypto Developers
 cpufeatures,https://github.com/RustCrypto/utils,MIT OR Apache-2.0,RustCrypto Developers
-either,https://github.com/rayon-rs/either,MIT OR Apache-2.0,bluss
+either,https://github.com/rayon-rs/either,MIT OR Apache-2.0,The either Authors
 equivalent,https://github.com/indexmap-rs/equivalent,Apache-2.0 OR MIT,The equivalent Authors
 foldhash,https://github.com/orlp/foldhash,Zlib,Orson Peters <orsonpeters@gmail.com>
 futures-core,https://github.com/rust-lang/futures-rs,MIT OR Apache-2.0,The futures-core Authors
 futures-sink,https://github.com/rust-lang/futures-rs,MIT OR Apache-2.0,The futures-sink Authors
 getrandom,https://github.com/rust-random/getrandom,MIT OR Apache-2.0,The Rand Project Developers
 hashbrown,https://github.com/rust-lang/hashbrown,MIT OR Apache-2.0,Amanieu d'Antras <amanieu@gmail.com>
+hashbrown,https://github.com/rust-lang/hashbrown,MIT OR Apache-2.0,The hashbrown Authors
 heck,https://github.com/withoutboats/heck,MIT OR Apache-2.0,The heck Authors
 id-arena,https://github.com/fitzgen/id-arena,MIT OR Apache-2.0,"Nick Fitzgerald <fitzgen@gmail.com>, Aleksey Kladov <aleksey.kladov@gmail.com>"
 indexmap,https://github.com/indexmap-rs/indexmap,Apache-2.0 OR MIT,The indexmap Authors

chitchat-test/Cargo.toml

@@ -7,7 +7,7 @@ rust-version = "1.86"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-chitchat = { version = "0.10.1", path = "../chitchat" }
+chitchat = { version = "0.11.0", path = "../chitchat" }
 poem = "3.1.11"
 poem-openapi = {version="5.1.15", features = ["swagger-ui"] }
 structopt = "0.3"

chitchat/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "chitchat"
-version = "0.10.1"
+version = "0.11.0"
 edition = "2024"
 license = "MIT"
 authors = ["Quickwit, Inc. <hello@quickwit.io>"]
@@ -17,7 +17,7 @@ bytes = "1"
 itertools = "0.14"
 lru = "0.17"
 rand = "0.10"
-serde = { version = "1", features = ["derive"] }
+serde = { version = "1", features = ["derive", "rc"] }
 tokio = { version = "1.28.0", features = [
     "net",
     "sync",

chitchat/src/failure_detector.rs

@@ -361,7 +361,7 @@ mod tests {
 
         let mut live_nodes = failure_detector
             .live_nodes()
-            .map(|chitchat_id| chitchat_id.node_id.as_str())
+            .map(|chitchat_id| &*chitchat_id.node_id)
             .collect::<Vec<_>>();
         live_nodes.sort_unstable();
         assert_eq!(live_nodes, vec!["node-10001", "node-10002", "node-10003"]);
@@ -374,7 +374,7 @@ mod tests {
         }
         let mut dead_nodes = failure_detector
             .dead_nodes()
-            .map(|chitchat_id| chitchat_id.node_id.as_str())
+            .map(|chitchat_id| &*chitchat_id.node_id)
             .collect::<Vec<_>>();
         dead_nodes.sort_unstable();
         assert_eq!(dead_nodes, vec!["node-10001", "node-10002", "node-10003"]);
@@ -386,20 +386,20 @@ mod tests {
         assert_eq!(
             failure_detector
                 .live_nodes()
-                .map(|chitchat_id| chitchat_id.node_id.as_str())
+                .map(|chitchat_id| &*chitchat_id.node_id)
                 .collect::<Vec<_>>(),
             Vec::<&str>::new()
         );
         assert_eq!(
             failure_detector
                 .dead_nodes()
-                .map(|chitchat_id| chitchat_id.node_id.as_str())
+                .map(|chitchat_id| &*chitchat_id.node_id)
                 .collect::<Vec<_>>(),
             Vec::<&str>::new()
         );
         let mut removed_nodes = garbage_collected_nodes
             .iter()
-            .map(|chitchat_id| chitchat_id.node_id.as_str())
+            .map(|chitchat_id| &*chitchat_id.node_id)
             .collect::<Vec<_>>();
         removed_nodes.sort_unstable();
         assert_eq!(
@@ -426,7 +426,7 @@ mod tests {
         assert_eq!(
             failure_detector
                 .live_nodes()
-                .map(|chitchat_id| chitchat_id.node_id.as_str())
+                .map(|chitchat_id| &*chitchat_id.node_id)
                 .collect::<Vec<_>>(),
             vec!["node-10001"]
         );
@@ -437,7 +437,7 @@ mod tests {
         assert_eq!(
             failure_detector
                 .live_nodes()
-                .map(|chitchat_id| chitchat_id.node_id.as_str())
+                .map(|chitchat_id| &*chitchat_id.node_id)
                 .collect::<Vec<_>>(),
             Vec::<&str>::new()
         );
@@ -452,7 +452,7 @@ mod tests {
         assert_eq!(
             failure_detector
                 .live_nodes()
-                .map(|chitchat_id| chitchat_id.node_id.as_str())
+                .map(|chitchat_id| &*chitchat_id.node_id)
                 .collect::<Vec<_>>(),
             vec!["node-10001"]
         );
@@ -479,7 +479,7 @@ mod tests {
 
         let live_nodes = failure_detector
             .live_nodes()
-            .map(|chitchat_id| chitchat_id.node_id.as_str())
+            .map(|chitchat_id| &*chitchat_id.node_id)
             .collect::<Vec<_>>();
         assert_eq!(live_nodes, vec!["node-10001"]);
 
@@ -488,7 +488,7 @@ mod tests {
 
         let live_nodes = failure_detector
             .live_nodes()
-            .map(|chitchat_id| chitchat_id.node_id.as_str())
+            .map(|chitchat_id| &*chitchat_id.node_id)
             .collect::<Vec<_>>();
         assert_eq!(live_nodes, Vec::<&str>::new());
     }

chitchat/src/lib.rs

@@ -963,7 +963,7 @@ mod tests {
 
         // Restart node at localhost:40001 with new name
         let mut new_config = ChitchatConfig::for_test(40_001);
-        new_config.chitchat_id.node_id = "new_node".to_string();
+        new_config.chitchat_id.node_id = Arc::from("new_node");
         let new_chitchat_id = new_config.chitchat_id.clone();
         let seed_addr = ChitchatId::for_local_test(40_002).gossip_advertise_addr;
         new_config.seed_nodes = vec![seed_addr.to_string()];
@@ -1354,7 +1354,7 @@ mod tests {
 
         fn id(i: usize) -> ChitchatId {
             ChitchatId {
-                node_id: "a".to_string().repeat(1000),
+                node_id: Arc::from("a".repeat(1000).as_str()),
                 generation_id: i as u64,
                 gossip_advertise_addr: SocketAddr::from(([127, 0, 0, 1], 10000u16 + i as u16)),
             }

chitchat/src/serialize.rs

@@ -1,5 +1,6 @@
 use std::io::BufRead;
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
+use std::sync::Arc;
 
 use anyhow::{Context, bail};
 use bytes::Buf;
@@ -183,18 +184,28 @@ impl Serializable for String {
     }
 }
 
+fn deserialize_str<'a>(buf: &mut &'a [u8]) -> anyhow::Result<&'a str> {
+    let len: usize = u16::deserialize(buf)? as usize;
+    let str_bytes = buf.get(..len).with_context(|| {
+        format!(
+            "failed to deserialize string, buffer too short (str_len={len}, buf_len={})",
+            buf.len()
+        )
+    })?;
+    let s = std::str::from_utf8(str_bytes)?;
+    buf.consume(len);
+    Ok(s)
+}
+
 impl Deserializable for String {
     fn deserialize(buf: &mut &[u8]) -> anyhow::Result<Self> {
-        let len: usize = u16::deserialize(buf)? as usize;
-        let str_bytes = buf.get(..len).with_context(|| {
-            format!(
-                "failed to deserialize string, buffer too short (str_len={len}, buf_len={})",
-                buf.len()
-            )
-        })?;
-        let str = std::str::from_utf8(str_bytes)?.to_string();
-        buf.consume(len);
-        Ok(str)
+        Ok(deserialize_str(buf)?.to_owned())
+    }
+}
+
+impl Deserializable for Arc<str> {
+    fn deserialize(buf: &mut &[u8]) -> anyhow::Result<Self> {
+        Ok(Arc::from(deserialize_str(buf)?))
     }
 }
 
@@ -264,7 +275,7 @@ impl Serializable for ChitchatId {
 
 impl Deserializable for ChitchatId {
     fn deserialize(buf: &mut &[u8]) -> anyhow::Result<Self> {
-        let node_id = String::deserialize(buf)?;
+        let node_id: Arc<str> = Arc::<str>::deserialize(buf)?;
         let generation_id = u64::deserialize(buf)?;
         let gossip_advertise_addr = SocketAddr::deserialize(buf)?;
         Ok(Self {

chitchat/src/state.rs

@@ -4,6 +4,7 @@ use std::collections::{BTreeMap, HashSet};
 use std::fmt::{Debug, Formatter};
 use std::net::{Ipv4Addr, SocketAddr};
 use std::ops::Bound;
+use std::sync::Arc;
 use std::time::Duration;
 
 use itertools::Itertools;
@@ -97,7 +98,7 @@ impl NodeState {
     pub fn for_test() -> NodeState {
         NodeState {
             chitchat_id: ChitchatId {
-                node_id: "test-node".to_string(),
+                node_id: Arc::from("test-node"),
                 generation_id: 0,
                 gossip_advertise_addr: SocketAddr::new(Ipv4Addr::new(127, 0, 0, 1).into(), 7280),
             },

chitchat/src/types.rs

@@ -1,5 +1,6 @@
 use std::fmt::Debug;
 use std::net::SocketAddr;
+use std::sync::Arc;
 
 use serde::{Deserialize, Serialize};
 use tokio::time::Instant;
@@ -20,7 +21,7 @@ use crate::serialize::Deserializable;
 #[derive(Clone, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize, Deserialize)]
 pub struct ChitchatId {
     /// An identifier unique across the cluster.
-    pub node_id: String,
+    pub node_id: Arc<str>,
     /// A numeric identifier incremented every time the node leaves and rejoins the cluster.
     pub generation_id: u64,
     /// The socket address peers should use to gossip with the node.
@@ -32,17 +33,19 @@ impl Debug for ChitchatId {
         write!(
             f,
             "{}:{}:{}",
-            self.node_id.as_str(),
-            self.generation_id,
-            self.gossip_advertise_addr
+            &*self.node_id, self.generation_id, self.gossip_advertise_addr
         )
     }
 }
 
 impl ChitchatId {
-    pub fn new(node_id: String, generation_id: u64, gossip_advertise_addr: SocketAddr) -> Self {
+    pub fn new(
+        node_id: impl Into<Arc<str>>,
+        generation_id: u64,
+        gossip_advertise_addr: SocketAddr,
+    ) -> Self {
         Self {
-            node_id,
+            node_id: node_id.into(),
             generation_id,
             gossip_advertise_addr,
         }

chitchat/tests/cluster_test.rs

@@ -1,6 +1,6 @@
 use std::collections::{HashMap, HashSet};
 use std::net::{SocketAddr, TcpListener};
-use std::sync::{Mutex, OnceLock};
+use std::sync::{Arc, Mutex, OnceLock};
 use std::time::Duration;
 
 use anyhow::anyhow;
@@ -445,15 +445,15 @@ impl Simulator {
 pub fn create_chitchat_id(id: &str) -> ChitchatId {
     let port = find_available_tcp_port().unwrap();
     ChitchatId {
-        node_id: id.to_string(),
+        node_id: Arc::from(id),
         generation_id: 0,
         gossip_advertise_addr: ([127, 0, 0, 1], port).into(),
     }
 }
 
 pub fn test_chitchat_id(port: u16) -> ChitchatId {
     ChitchatId {
-        node_id: format!("node_{port}"),
+        node_id: Arc::from(format!("node_{port}").as_str()),
         generation_id: 0,
         gossip_advertise_addr: ([127, 0, 0, 1], port).into(),
     }

chitchat/tests/perf_test.rs

@@ -1,5 +1,6 @@
 use std::collections::BTreeMap;
 use std::net::SocketAddr;
+use std::sync::Arc;
 use std::time::Duration;
 
 use chitchat::transport::{ChannelTransport, Transport, TransportExt};
@@ -13,7 +14,7 @@ use tracing::info;
 async fn spawn_one(chitchat_id: u16, transport: &dyn Transport) -> ChitchatHandle {
     let listen_addr: SocketAddr = ([127, 0, 0, 1], 10_000u16 + chitchat_id).into();
     let chitchat_id = ChitchatId {
-        node_id: format!("node_{chitchat_id}"),
+        node_id: Arc::from(format!("node_{chitchat_id}").as_str()),
         generation_id: 0,
         gossip_advertise_addr: listen_addr,
     };