diff --git a/CHANGELOG.md b/CHANGELOG.md index 07adae8368a1b9527b3eff889d49af695c3ccbb2..9adc11cfa58c9b1171c8a48b7547d98cf99e6773 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ with the `YY.0M.MICRO` scheme. ### Features +- New property in tier table - `can_vote`. Indicates whether instances from tier can be considered as voter in raft protocol. + - Transferred replication factor from Properties table to new Tier table. Each tier has his own replication factor. - New feature `tier` - group of instances with own replication factor. diff --git a/src/governor/cc.rs b/src/governor/cc.rs index 1fb8b3939afc768832c7a580a44c6f563589abf3..98a65c2ecb675be760d483a9ee3e1f354f55eec5 100644 --- a/src/governor/cc.rs +++ b/src/governor/cc.rs @@ -1,11 +1,12 @@ use ::raft::prelude as raft; use ::raft::prelude::ConfChangeType::*; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use crate::has_grades; use crate::instance::GradeVariant::*; use crate::instance::Instance; +use crate::tier::Tier; use crate::traft::{Distance, RaftId}; struct RaftConf<'a> { @@ -74,6 +75,7 @@ pub(crate) fn raft_conf_change( instances: &[Instance], voters: &[RaftId], learners: &[RaftId], + tiers: &HashMap<&String, &Tier>, ) -> Option<raft::ConfChangeV2> { let mut raft_conf = RaftConf { all: instances.iter().map(|p| (p.raft_id, p)).collect(), @@ -83,16 +85,23 @@ pub(crate) fn raft_conf_change( let mut changes: Vec<raft::ConfChangeSingle> = vec![]; let not_expelled = |instance: &&Instance| has_grades!(instance, * -> not Expelled); + // Only an instance from tier with `can_vote=true` can be considered as voter. + let eligible_for_vote = |instance: &&Instance| { + tiers + .get(&instance.tier) + .expect("tier for instance should exists") + .can_vote + }; + + let number_of_eligible_for_vote = instances + .iter() + .filter(not_expelled) + .filter(eligible_for_vote) + .count(); - let cluster_size = instances.iter().filter(not_expelled).count(); - let voters_needed = match cluster_size { - // five and more nodes -> 5 voters + let voters_needed = match number_of_eligible_for_vote { 5.. => 5, - // three or four nodes -> 3 voters 3..=4 => 3, - // two nodes -> 2 voters - // one node -> 1 voter - // zero nodes -> 0 voters (almost unreachable) x => x, }; @@ -101,6 +110,7 @@ pub(crate) fn raft_conf_change( .iter() // Only an instance with current grade online can be promoted .filter(|instance| has_grades!(instance, Online -> Online)) + .filter(eligible_for_vote) .map(|instance| instance.raft_id) // Exclude those who is already a voter. .filter(|raft_id| !raft_conf.voters.contains(raft_id)) @@ -112,11 +122,20 @@ pub(crate) fn raft_conf_change( // Remove / replace voters for voter_id in raft_conf.voters.clone().iter() { - let Some(instance) = raft_conf.all.get(voter_id) else { - // Nearly impossible, but rust forces me to check it. - let ccs = raft_conf.change_single(RemoveNode, *voter_id); - changes.push(ccs); - continue; + let instance = match raft_conf.all.get(voter_id) { + Some(instance) if eligible_for_vote(instance) => instance, + // case when bootstrap leader from tier with `can_vote`: false + Some(_) => { + let ccs = raft_conf.change_single(RemoveNode, *voter_id); + changes.push(ccs); + continue; + } + // unknown instance which in not in configuration of cluster, nearly impossible + _ => { + let ccs = raft_conf.change_single(RemoveNode, *voter_id); + changes.push(ccs); + continue; + } }; match instance.target_grade.variant { Online => { @@ -227,6 +246,7 @@ pub(crate) fn raft_conf_change( #[cfg(test)] mod tests { use super::*; + use crate::tier::DEFAULT_TIER; use ::raft::prelude as raft; use std::collections::{BTreeMap, BTreeSet}; @@ -250,6 +270,7 @@ mod tests { ) => { Instance { raft_id: $raft_id, + tier: DEFAULT_TIER.into(), current_grade: Grade { variant: $current_grade, // raft_conf_change doesn't care about incarnations @@ -275,6 +296,19 @@ mod tests { }; } + macro_rules! p_with_tier { + ( + $raft_id:literal, + $tier:literal, + $grade:ident + + ) => {{ + let mut instance = p!($raft_id, $grade -> $grade); + instance.tier = $tier.to_string(); + instance + }} + } + macro_rules! cc { [$( $change:ident($raft_id:literal) @@ -294,7 +328,19 @@ mod tests { } fn cc(p: &[Instance], v: &[RaftId], l: &[RaftId]) -> Option<raft::ConfChangeV2> { - let mut cc = super::raft_conf_change(p, v, l)?; + let mut t = HashMap::new(); + let tier = Tier::default(); + t.insert(&tier.name, &tier); + cc_with_tiers(p, v, l, &t) + } + + fn cc_with_tiers( + p: &[Instance], + v: &[RaftId], + l: &[RaftId], + t: &HashMap<&String, &Tier>, + ) -> Option<raft::ConfChangeV2> { + let mut cc = super::raft_conf_change(p, v, l, t)?; cc.changes.sort_by(|l, r| Ord::cmp(&l.node_id, &r.node_id)); Some(cc) } @@ -555,5 +601,47 @@ mod tests { // Vouters number should not fall below the optimal number cc![AddLearnerNode(1), AddNode(4)] ); + + let mut tiers = HashMap::new(); + let storage_tier = Tier { + name: "storage".into(), + replication_factor: 1, + can_vote: true, + }; + let router_tier = Tier { + name: "router".into(), + replication_factor: 1, + can_vote: false, + }; + tiers.insert(&storage_tier.name, &storage_tier); + tiers.insert(&router_tier.name, &router_tier); + + assert_eq!( + cc_with_tiers( + &[ + p_with_tier!(1, "storage", Online), + p_with_tier!(2, "router", Online), + ], + &[1], + &[2], + &tiers + ), + // instance from router tier couldn't be upgraded to voter + None + ); + + assert_eq!( + cc_with_tiers( + &[ + p_with_tier!(1, "storage", Online), + p_with_tier!(2, "router", Online), + ], + &[2], + &[1], + &tiers + ), + // case when bootstrap leader from tier with falsy `can_vote` + cc![AddNode(1), RemoveNode(2), AddLearnerNode(2)] + ); } } diff --git a/src/governor/plan.rs b/src/governor/plan.rs index 98db294295e7b6b4376a7f455a5d264cb3711aa3..6de984038b96d88c9cd636a0f661a049bbf5cca6 100644 --- a/src/governor/plan.rs +++ b/src/governor/plan.rs @@ -34,7 +34,7 @@ pub(super) fn action_plan<'i>( ) -> Result<Plan<'i>> { //////////////////////////////////////////////////////////////////////////// // conf change - if let Some(conf_change) = raft_conf_change(instances, voters, learners) { + if let Some(conf_change) = raft_conf_change(instances, voters, learners, tiers) { return Ok(Plan::ConfChange(ConfChange { conf_change })); } diff --git a/src/instance.rs b/src/instance.rs index 138c192be17dbe539fbb2c55ebb64fef6d88892a..b7f8d86897e236d1d7b8a4505eb938802a736a29 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -146,6 +146,7 @@ mod tests { &Tier { name: name.into(), replication_factor, + can_vote: true, } ) } @@ -159,6 +160,7 @@ mod tests { &Tier { name: DEFAULT_TIER.into(), replication_factor, + can_vote: true, } ).unwrap(); } diff --git a/src/storage.rs b/src/storage.rs index 25f56cb36145307bb71383b0ee8968b95c25caa0..d0a8cb8810a65c6f97aaf9ae4721a324139d7650 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -2590,6 +2590,7 @@ impl Tiers { .space_type(SpaceType::DataLocal) .field(("name", FieldType::String)) .field(("replication_factor", FieldType::Unsigned)) + .field(("can_vote", FieldType::Boolean)) .if_not_exists(true) .create()?; diff --git a/src/tier.rs b/src/tier.rs index 47135ed0b30203f1137b886f1f5a6531ba85f23c..7d33fcf5392b68a65f62a6d817cd6d039b816adb 100644 --- a/src/tier.rs +++ b/src/tier.rs @@ -9,6 +9,8 @@ pub const DEFAULT_TIER: &str = "default"; pub struct Tier { pub name: String, pub replication_factor: u8, + // Indicates whether instances from tier could be considered as voter in raft leader election. + pub can_vote: bool, } impl Default for Tier { @@ -16,6 +18,7 @@ impl Default for Tier { Tier { name: DEFAULT_TIER.into(), replication_factor: 1, + can_vote: true, } } } @@ -25,6 +28,7 @@ impl Tier { Tier { name: DEFAULT_TIER.into(), replication_factor, + can_vote: true, } } } diff --git a/test/int/test_basics.py b/test/int/test_basics.py index fb116afd2313d1fddef01c9b22e99bfba6a26695..deaf511224bc8d056057d65fd832b557a2cf5bcf 100644 --- a/test/int/test_basics.py +++ b/test/int/test_basics.py @@ -180,8 +180,8 @@ def test_whoami(instance: Instance): def test_whoami_in_different_tiers(cluster: Cluster): cfg = { "tiers": [ - {"name": "storage", "replication_factor": 1}, - {"name": "router", "replication_factor": 2}, + {"name": "storage", "replication_factor": 1, "can_vote": True}, + {"name": "router", "replication_factor": 2, "can_vote": True}, ] } @@ -242,7 +242,7 @@ def test_raft_log(instance: Instance): +-----+----+-----+--------+ | 1 | 1 |1.0.1|Insert({_pico_peer_address}, [1,"127.0.0.1:{p}"])| | 2 | 1 |1.0.2|Insert({_pico_instance}, ["i1","68d4a766-4144-3248-aeb4-e212356716e4",1,"r1","e0df68c5-e7f9-395f-86b3-30ad9e1b7b07",["Offline",0],["Offline",0],{b},"default"])| -| 3 | 1 |1.0.3|Insert(523, ["default",1])| +| 3 | 1 |1.0.3|Insert(523, ["default",1,true])| | 4 | 1 |1.0.4|Insert({_pico_property}, ["global_schema_version",0])| | 5 | 1 |1.0.5|Insert({_pico_property}, ["next_schema_version",1])| | 6 | 1 |1.0.6|Insert({_pico_property}, ["password_min_length",8])| diff --git a/test/int/test_init_cfg.py b/test/int/test_init_cfg.py index e7a5b3a899ef4f878c99052be619f8b3aeaa2b78..30e06b3907b7cde448b1a156701d1b350f27595c 100644 --- a/test/int/test_init_cfg.py +++ b/test/int/test_init_cfg.py @@ -19,8 +19,8 @@ from ./unexisting_dir/trash.yaml, error: No such file or directory (os error 2)\ def test_run_init_cfg_with_duplicated_tier_names(cluster: Cluster): cfg = { "tiers": [ - {"name": "storage", "replication_factor": 1}, - {"name": "storage", "replication_factor": 2}, + {"name": "storage", "replication_factor": 1, "can_vote": True}, + {"name": "storage", "replication_factor": 2, "can_vote": True}, ] } diff --git a/test/int/test_shutdown.py b/test/int/test_shutdown.py index 7fd66823c036736d0e10c05041f7c2856bb4e692..a23cb8b1e46bae07c53b5d0a4f63b35ab3e05442 100644 --- a/test/int/test_shutdown.py +++ b/test/int/test_shutdown.py @@ -112,3 +112,28 @@ def test_threesome(cluster3: Cluster): c2 = log_crawler(i2, ON_SHUTDOWN_TIMEOUT) i2.terminate(kill_after_seconds=1) assert not c2.matched + + +def test_instance_from_falsy_tier_is_not_voter(cluster: Cluster): + cfg = { + "tiers": [ + {"name": "storage", "replication_factor": 1, "can_vote": True}, + {"name": "router", "replication_factor": 1, "can_vote": False}, + ] + } + cluster.set_init_cfg(cfg) + i1 = cluster.add_instance(tier="storage") + i2 = cluster.add_instance(tier="router") + c1 = log_crawler( + i1, + "leader is going offline and no substitution is found, voters: [1], leader_raft_id: 1", + ) + + # make sure i1 is leader + i1.assert_raft_status("Leader") + i2.assert_raft_status("Follower", leader_id=i1.raft_id) + + i1.terminate(kill_after_seconds=1) + + i2.assert_raft_status("Follower", leader_id=i1.raft_id) + assert c1.matched