From 05d6827fe8e3f73d64a2682ecdb5ffaad8ac51b9 Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Fri, 1 Jul 2022 20:28:01 +0300
Subject: [PATCH] feat: store failure domains in raft_group + non working test

---
 src/traft/mod.rs      |  6 +++
 src/traft/storage.rs  | 25 +++++++----
 src/traft/topology.rs | 97 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 113 insertions(+), 15 deletions(-)

diff --git a/src/traft/mod.rs b/src/traft/mod.rs
index 3aa5001ed6..0df2c296d7 100644
--- a/src/traft/mod.rs
+++ b/src/traft/mod.rs
@@ -149,6 +149,12 @@ pub struct Peer {
 
     /// The state of this instance's activity.
     pub health: Health,
+
+    /// Instance failure domains. Instances with overlapping failure domains
+    /// must not be in the same replicaset.
+    // TODO: raft_group space is kinda bloated, maybe we should store some data
+    // in different spaces/not deserialize the whole tuple every time?
+    pub failure_domains: FailureDomains,
 }
 impl AsTuple for Peer {}
 
diff --git a/src/traft/storage.rs b/src/traft/storage.rs
index 0d28cb6bd4..9a6e20e292 100644
--- a/src/traft/storage.rs
+++ b/src/traft/storage.rs
@@ -84,6 +84,7 @@ impl Storage {
                     {name = 'replicaset_uuid', type = 'string', is_nullable = false},
                     {name = 'commit_index', type = 'unsigned', is_nullable = false},
                     {name = 'health', type = 'string', is_nullable = false},
+                    {name = 'failure_domains', type = 'map', is_nullable = false},
                 }
             })
 
@@ -597,17 +598,27 @@ inventory::submit!(crate::InnerTest {
 
         let mut raft_group = Storage::space(RAFT_GROUP).unwrap();
 
+        let faildom = crate::traft::FailureDomains::from([("a", "b")]);
+
         for peer in vec![
             // r1
             (
-                "i1", "i1-uuid", 1u64, "addr:1", "r1", "r1-uuid", 1u64, Online,
+                "i1", "i1-uuid", 1u64, "addr:1", "r1", "r1-uuid", 1u64, Online, &faildom,
+            ),
+            (
+                "i2", "i2-uuid", 2u64, "addr:2", "r1", "r1-uuid", 2, Online, &faildom,
             ),
-            ("i2", "i2-uuid", 2u64, "addr:2", "r1", "r1-uuid", 2, Online),
             // r2
-            ("i3", "i3-uuid", 3u64, "addr:3", "r2", "r2-uuid", 10, Online),
-            ("i4", "i4-uuid", 4u64, "addr:4", "r2", "r2-uuid", 10, Online),
+            (
+                "i3", "i3-uuid", 3u64, "addr:3", "r2", "r2-uuid", 10, Online, &faildom,
+            ),
+            (
+                "i4", "i4-uuid", 4u64, "addr:4", "r2", "r2-uuid", 10, Online, &faildom,
+            ),
             // r3
-            ("i5", "i5-uuid", 5u64, "addr:5", "r3", "r3-uuid", 10, Online),
+            (
+                "i5", "i5-uuid", 5u64, "addr:5", "r3", "r3-uuid", 10, Online, &faildom,
+            ),
         ] {
             raft_group.put(&peer).unwrap();
         }
@@ -632,9 +643,9 @@ inventory::submit!(crate::InnerTest {
                     " in unique index \"raft_id\"",
                     " in space \"raft_group\"",
                     " with old tuple",
-                    r#" - ["i1", "i1-uuid", 1, "addr:1", "r1", "r1-uuid", 1, "{on}"]"#,
+                    r#" - ["i1", "i1-uuid", 1, "addr:1", "r1", "r1-uuid", 1, "{on}", {{"A": "B"}}]"#,
                     " and new tuple",
-                    r#" - ["i99", "", 1, "", "", "", 0, "{off}"]"#,
+                    r#" - ["i99", "", 1, "", "", "", 0, "{off}", {{}}]"#,
                 ),
                 on = Online,
                 off = Offline,
diff --git a/src/traft/topology.rs b/src/traft/topology.rs
index 948a4f129e..a5a7089b4e 100644
--- a/src/traft/topology.rs
+++ b/src/traft/topology.rs
@@ -106,9 +106,6 @@ impl Topology {
             replicaset_id.unwrap_or_else(|| self.choose_replicaset_id(&failure_domains));
         let replicaset_uuid = replicaset_uuid(&replicaset_id);
 
-        // TODO: store it in peer
-        let _ = failure_domains;
-
         let peer = Peer {
             instance_id,
             instance_uuid,
@@ -121,6 +118,7 @@ impl Topology {
             // It prevents a disruption in case of the
             // instance_id collision.
             health: Health::Online,
+            failure_domains,
         };
 
         self.put_peer(peer.clone());
@@ -185,10 +183,12 @@ mod tests {
             $instance_id:literal,
             $replicaset_id:literal,
             $peer_address:literal,
-            $health:expr $(,)?
+            $health:expr
+            $(, $failure_domains:expr)?
+            $(,)?
         ) ),* $(,)? ] => {
             vec![$(
-                peer!($raft_id, $instance_id, $replicaset_id, $peer_address, $health)
+                peer!($raft_id, $instance_id, $replicaset_id, $peer_address, $health $(,$failure_domains)?)
             ),*]
         };
     }
@@ -199,7 +199,9 @@ mod tests {
             $instance_id:literal,
             $replicaset_id:literal,
             $peer_address:literal,
-            $health:expr $(,)?
+            $health:expr
+            $(, $failure_domains:expr)?
+            $(,)?
         ) => {
             Peer {
                 raft_id: $raft_id,
@@ -210,6 +212,11 @@ mod tests {
                 replicaset_uuid: replicaset_uuid($replicaset_id),
                 commit_index: raft::INVALID_INDEX,
                 health: $health,
+                failure_domains: {
+                    let _f = FailureDomains::default();
+                    $( let _f = $failure_domains; )?
+                    _f
+                },
             }
         };
     }
@@ -219,17 +226,29 @@ mod tests {
             $topology:expr,
             $instance_id:expr,
             $replicaset_id:expr,
-            $advertise_address:literal $(,)?
+            $advertise_address:literal
+            $(, $failure_domains:expr )?
+            $(,)?
         ) => {
             $topology.join(
                 $instance_id.map(str::to_string),
                 $replicaset_id.map(str::to_string),
                 $advertise_address.into(),
-                FailureDomains::default(),
+                {
+                    let _f = FailureDomains::default();
+                    $(let _f = $failure_domains; )?
+                    _f
+                },
             )
         };
     }
 
+    macro_rules! faildoms {
+        ($($k:tt : $v:tt),* $(,)?) => {
+            FailureDomains::from([$((stringify!($k), stringify!($v))),*])
+        }
+    }
+
     #[test]
     fn test_simple() {
         let mut topology = Topology::from_peers(vec![]).with_replication_factor(1);
@@ -379,4 +398,66 @@ mod tests {
             peer!(2, "i2", "r2", "nowhere", Online),
         );
     }
+
+    #[test]
+    #[should_panic]
+    fn failure_domains() {
+        let mut t = Topology::from_peers(peers![]).with_replication_factor(3);
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Earth})
+                .unwrap()
+                .replicaset_id,
+            "r1",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Earth})
+                .unwrap()
+                .replicaset_id,
+            "r2",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Mars})
+                .unwrap()
+                .replicaset_id,
+            "r1",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Earth, os: BSD})
+                .unwrap()
+                .replicaset_id,
+            "r3",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Mars, os: BSD})
+                .unwrap()
+                .replicaset_id,
+            "r2",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {os: Arch})
+                .unwrap()
+                .replicaset_id,
+            "r2",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {planet: Venus, os: Arch})
+                .unwrap()
+                .replicaset_id,
+            "r1",
+        );
+
+        assert_eq!(
+            join!(t, None, None, "-", faildoms! {os: Mac})
+                .unwrap()
+                .replicaset_id,
+            "r3",
+        );
+    }
 }
-- 
GitLab