From 6ffd91d4067aac015037c1e61d91d4fabfc7f46a Mon Sep 17 00:00:00 2001
From: Kurdakov Alexander <kusancho12@gmail.com>
Date: Fri, 27 Oct 2023 03:02:19 +0300
Subject: [PATCH] feat: add can_vote property to tier entity
New property in configuration of tier - can_vote. Indicates whether instances from tier could be considered as voter in raft leader election.
---
CHANGELOG.md | 2 +
src/governor/cc.rs | 116 +++++++++++++++++++++++++++++++++-----
src/governor/plan.rs | 2 +-
src/instance.rs | 2 +
src/storage.rs | 1 +
src/tier.rs | 4 ++
test/int/test_basics.py | 6 +-
test/int/test_init_cfg.py | 4 +-
test/int/test_shutdown.py | 25 ++++++++
9 files changed, 142 insertions(+), 20 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07adae8368..9adc11cfa5 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 1fb8b3939a..98a65c2ecb 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 98db294295..6de984038b 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 138c192be1..b7f8d86897 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 25f56cb361..d0a8cb8810 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 47135ed0b3..7d33fcf539 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 fb116afd23..deaf511224 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 e7a5b3a899..30e06b3907 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 7fd66823c0..a23cb8b1e4 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
--
GitLab