From a6215e86fde413db093a1d00ea8c6fcd34373c05 Mon Sep 17 00:00:00 2001
From: Valentin Syrovatskiy <v.syrovatskiy@picodata.io>
Date: Tue, 30 Aug 2022 17:59:14 +0300
Subject: [PATCH] refactor: add Peer.target_grade
---
src/main.rs | 3 +-
src/traft/mod.rs | 39 +++++++++++++++++++++-
src/traft/storage.rs | 33 ++++++++-----------
src/traft/topology.rs | 71 ++++++++++++++++++++++------------------
test/int/test_couple.py | 2 +-
test/int/test_joining.py | 1 +
6 files changed, 95 insertions(+), 54 deletions(-)
diff --git a/src/main.rs b/src/main.rs
index ef345b8bef..05c8198580 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,7 +16,7 @@ use traft::ExpelRequest;
use clap::StructOpt as _;
use protobuf::Message as _;
-use crate::traft::{Grade, LogicalClock, RaftIndex, UpdatePeerRequest};
+use crate::traft::{Grade, LogicalClock, RaftIndex, TargetGrade, UpdatePeerRequest};
use traft::error::Error;
mod app;
@@ -812,6 +812,7 @@ fn postjoin(args: &args::Run) {
tlog!(Info, "initiating self-activation of {}", peer.instance_id);
let req = UpdatePeerRequest::new(peer.instance_id, cluster_id)
.with_grade(Grade::Online)
+ .with_target_grade(TargetGrade::Online)
.with_failure_domain(args.failure_domain());
let leader_id = node.status().leader_id.expect("leader_id deinitialized");
diff --git a/src/traft/mod.rs b/src/traft/mod.rs
index 462198308b..52b9d22eb6 100644
--- a/src/traft/mod.rs
+++ b/src/traft/mod.rs
@@ -198,8 +198,10 @@ pub struct Peer {
/// `0` means it's not committed yet.
pub commit_index: RaftIndex,
- /// The state of this instance's activity.
+ /// The cluster's mind about actual state of this instance's activity.
pub grade: Grade,
+ /// The desired state of this instance
+ pub target_grade: TargetGrade,
/// Instance failure domains. Instances with overlapping failure domains
/// must not be in the same replicaset.
@@ -606,6 +608,35 @@ impl Default for Grade {
}
}
+#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
+pub enum TargetGrade {
+ // Instance should be configured up
+ Online,
+ // Instance should be gracefully shut down
+ Offline,
+ // Instance should be removed from cluster
+ Expelled,
+}
+impl TargetGrade {
+ const fn to_str(&self) -> &str {
+ match self {
+ Self::Online => "Online",
+ Self::Offline => "Offline",
+ Self::Expelled => "Expelled",
+ }
+ }
+}
+impl Display for TargetGrade {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ f.write_str(self.to_str())
+ }
+}
+impl Default for TargetGrade {
+ fn default() -> Self {
+ Self::Online
+ }
+}
+
///////////////////////////////////////////////////////////////////////////////
/// Request to update the instance in the storage.
#[derive(Clone, Debug, Serialize, Deserialize)]
@@ -618,6 +649,7 @@ pub struct UpdatePeerRequest {
#[derive(Clone, Debug, Serialize, Deserialize)]
pub enum PeerChange {
Grade(Grade),
+ TargetGrade(TargetGrade),
FailureDomain(FailureDomain),
}
@@ -637,6 +669,11 @@ impl UpdatePeerRequest {
self
}
#[inline]
+ pub fn with_target_grade(mut self, target_grade: TargetGrade) -> Self {
+ self.changes.push(PeerChange::TargetGrade(target_grade));
+ self
+ }
+ #[inline]
pub fn with_failure_domain(mut self, failure_domain: FailureDomain) -> Self {
self.changes.push(PeerChange::FailureDomain(failure_domain));
self
diff --git a/src/traft/storage.rs b/src/traft/storage.rs
index 48d9f20b90..d8f7eb0691 100644
--- a/src/traft/storage.rs
+++ b/src/traft/storage.rs
@@ -86,6 +86,7 @@ impl Storage {
{name = 'replicaset_uuid', type = 'string', is_nullable = false},
{name = 'commit_index', type = 'unsigned', is_nullable = false},
{name = 'grade', type = 'string', is_nullable = false},
+ {name = 'target_grade', type = 'string', is_nullable = false},
{name = 'failure_domain', type = 'map', is_nullable = false},
}
})
@@ -605,10 +606,11 @@ inventory::submit!(crate::InnerTest {
}
});
+#[rustfmt::skip]
inventory::submit!(crate::InnerTest {
name: "test_storage_peers",
body: || {
- use traft::Grade::{Offline, Online};
+ use traft::{Grade, TargetGrade};
let mut raft_group = Storage::space(RAFT_GROUP).unwrap();
@@ -616,23 +618,13 @@ inventory::submit!(crate::InnerTest {
for peer in vec![
// r1
- (
- "i1", "i1-uuid", 1u64, "addr:1", "r1", "r1-uuid", 1u64, Online, &faildom,
- ),
- (
- "i2", "i2-uuid", 2u64, "addr:2", "r1", "r1-uuid", 2, Online, &faildom,
- ),
+ ("i1", "i1-uuid", 1u64, "addr:1", "r1", "r1-uuid", 1u64, Grade::Online, TargetGrade::Online, &faildom,),
+ ("i2", "i2-uuid", 2u64, "addr:2", "r1", "r1-uuid", 2, Grade::Online, TargetGrade::Online, &faildom,),
// r2
- (
- "i3", "i3-uuid", 3u64, "addr:3", "r2", "r2-uuid", 10, Online, &faildom,
- ),
- (
- "i4", "i4-uuid", 4u64, "addr:4", "r2", "r2-uuid", 10, Online, &faildom,
- ),
+ ("i3", "i3-uuid", 3u64, "addr:3", "r2", "r2-uuid", 10, Grade::Online, TargetGrade::Online, &faildom,),
+ ("i4", "i4-uuid", 4u64, "addr:4", "r2", "r2-uuid", 10, Grade::Online, TargetGrade::Online, &faildom,),
// r3
- (
- "i5", "i5-uuid", 5u64, "addr:5", "r3", "r3-uuid", 10, Online, &faildom,
- ),
+ ("i5", "i5-uuid", 5u64, "addr:5", "r3", "r3-uuid", 10, Grade::Online, TargetGrade::Online, &faildom,),
] {
raft_group.put(&peer).unwrap();
}
@@ -657,12 +649,13 @@ 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}", {{"A": "B"}}]"#,
+ r#" - ["i1", "i1-uuid", 1, "addr:1", "r1", "r1-uuid", 1, "{gon}", "{tgon}", {{"A": "B"}}]"#,
" and new tuple",
- r#" - ["i99", "", 1, "", "", "", 0, "{off}", {{}}]"#,
+ r#" - ["i99", "", 1, "", "", "", 0, "{goff}", "{tgon}", {{}}]"#,
),
- on = Online,
- off = Offline,
+ gon = Grade::Online,
+ goff = Grade::Offline,
+ tgon = TargetGrade::Online,
)
);
diff --git a/src/traft/topology.rs b/src/traft/topology.rs
index d288492852..ecf3c54713 100644
--- a/src/traft/topology.rs
+++ b/src/traft/topology.rs
@@ -11,6 +11,8 @@ use crate::util::Uppercase;
use raft::INVALID_INDEX;
+use super::TargetGrade;
+
pub struct Topology {
replication_factor: u8,
max_raft_id: RaftId,
@@ -166,6 +168,7 @@ impl Topology {
// It prevents a disruption in case of the
// instance_id collision.
grade: Grade::Online,
+ target_grade: super::TargetGrade::Online,
failure_domain,
};
@@ -203,6 +206,11 @@ impl Topology {
peer.grade = grade;
}
}
+ PeerChange::TargetGrade(target_grade) => {
+ if peer.target_grade != TargetGrade::Expelled {
+ peer.target_grade = target_grade;
+ }
+ }
PeerChange::FailureDomain(fd) => {
// SAFETY: this is safe, because rust doesn't complain if you inline
// the function
@@ -239,9 +247,9 @@ mod tests {
use crate::traft::instance_uuid;
use crate::traft::replicaset_uuid;
use crate::traft::FailureDomain;
- use crate::traft::Grade::{Offline, Online};
use crate::traft::Peer;
use crate::traft::UpdatePeerRequest;
+ use crate::traft::{Grade, TargetGrade};
use pretty_assertions::assert_eq;
macro_rules! peers {
@@ -279,6 +287,7 @@ mod tests {
replicaset_uuid: replicaset_uuid($replicaset_id),
commit_index: raft::INVALID_INDEX,
grade: $grade,
+ target_grade: TargetGrade::Online,
failure_domain: {
let _f = FailureDomain::default();
$( let _f = $failure_domain; )?
@@ -330,7 +339,7 @@ mod tests {
) => {
$topology.update_peer(
UpdatePeerRequest::new($instance_id.into(), "".into())
- .with_grade(Online)
+ .with_grade(Grade::Online)
.with_failure_domain($failure_domain),
)
};
@@ -349,38 +358,38 @@ mod tests {
assert_eq!(
join!(topology, None, None, "addr:1").unwrap(),
- peer!(1, "i1", "r1", "addr:1", Online)
+ peer!(1, "i1", "r1", "addr:1", Grade::Online)
);
assert_eq!(
join!(topology, None, None, "addr:1").unwrap(),
- peer!(2, "i2", "r2", "addr:1", Online)
+ peer!(2, "i2", "r2", "addr:1", Grade::Online)
);
assert_eq!(
join!(topology, None, Some("R3"), "addr:1").unwrap(),
- peer!(3, "i3", "R3", "addr:1", Online)
+ peer!(3, "i3", "R3", "addr:1", Grade::Online)
);
assert_eq!(
join!(topology, Some("I4"), None, "addr:1").unwrap(),
- peer!(4, "I4", "r3", "addr:1", Online)
+ peer!(4, "I4", "r3", "addr:1", Grade::Online)
);
- let mut topology = Topology::from_peers(peers![(1, "i1", "r1", "addr:1", Online)])
+ let mut topology = Topology::from_peers(peers![(1, "i1", "r1", "addr:1", Grade::Online)])
.with_replication_factor(1);
assert_eq!(
join!(topology, None, None, "addr:1").unwrap(),
- peer!(2, "i2", "r2", "addr:1", Online)
+ peer!(2, "i2", "r2", "addr:1", Grade::Online)
);
}
#[test]
fn test_override() {
let mut topology = Topology::from_peers(peers![
- (1, "i1", "r1", "active:1", Online),
- (2, "i2", "r2-original", "inactive:1", Offline),
+ (1, "i1", "r1", "active:1", Grade::Online),
+ (2, "i2", "r2-original", "inactive:1", Grade::Offline),
])
.with_replication_factor(2);
@@ -408,7 +417,7 @@ mod tests {
// Disruption isn't destructive if auto-expel allows (TODO).
assert_eq!(
join!(topology, Some("i2"), None, "inactive:2").unwrap(),
- peer!(3, "i2", "r1", "inactive:2", Online),
+ peer!(3, "i2", "r1", "inactive:2", Grade::Online),
// Attention: generated replicaset_id differs from the
// original one, as well as raft_id.
// That's a desired behavior.
@@ -430,71 +439,71 @@ mod tests {
#[test]
fn test_instance_id_collision() {
let mut topology = Topology::from_peers(peers![
- (1, "i1", "r1", "addr:1", Online),
- (2, "i3", "r3", "addr:3", Online),
+ (1, "i1", "r1", "addr:1", Grade::Online),
+ (2, "i3", "r3", "addr:3", Grade::Online),
// Attention: i3 has raft_id=2
]);
assert_eq!(
join!(topology, None, Some("r2"), "addr:2").unwrap(),
- peer!(3, "i3-2", "r2", "addr:2", Online),
+ peer!(3, "i3-2", "r2", "addr:2", Grade::Online),
);
}
#[test]
fn test_replication_factor() {
let mut topology = Topology::from_peers(peers![
- (9, "i9", "r9", "nowhere", Online),
- (10, "i10", "r9", "nowhere", Online),
+ (9, "i9", "r9", "nowhere", Grade::Online),
+ (10, "i10", "r9", "nowhere", Grade::Online),
])
.with_replication_factor(2);
assert_eq!(
join!(topology, Some("i1"), None, "addr:1").unwrap(),
- peer!(11, "i1", "r1", "addr:1", Online),
+ peer!(11, "i1", "r1", "addr:1", Grade::Online),
);
assert_eq!(
join!(topology, Some("i2"), None, "addr:2").unwrap(),
- peer!(12, "i2", "r1", "addr:2", Online),
+ peer!(12, "i2", "r1", "addr:2", Grade::Online),
);
assert_eq!(
join!(topology, Some("i3"), None, "addr:3").unwrap(),
- peer!(13, "i3", "r2", "addr:3", Online),
+ peer!(13, "i3", "r2", "addr:3", Grade::Online),
);
assert_eq!(
join!(topology, Some("i4"), None, "addr:4").unwrap(),
- peer!(14, "i4", "r2", "addr:4", Online),
+ peer!(14, "i4", "r2", "addr:4", Grade::Online),
);
}
#[test]
fn test_set_active() {
let mut topology = Topology::from_peers(peers![
- (1, "i1", "r1", "nowhere", Online),
- (2, "i2", "r2", "nowhere", Offline),
+ (1, "i1", "r1", "nowhere", Grade::Online),
+ (2, "i2", "r2", "nowhere", Grade::Offline),
])
.with_replication_factor(1);
assert_eq!(
- set_grade!(topology, "i1", Offline).unwrap(),
- peer!(1, "i1", "r1", "nowhere", Offline),
+ set_grade!(topology, "i1", Grade::Offline).unwrap(),
+ peer!(1, "i1", "r1", "nowhere", Grade::Offline),
);
// idempotency
assert_eq!(
- set_grade!(topology, "i1", Offline).unwrap(),
- peer!(1, "i1", "r1", "nowhere", Offline),
+ set_grade!(topology, "i1", Grade::Offline).unwrap(),
+ peer!(1, "i1", "r1", "nowhere", Grade::Offline),
);
assert_eq!(
- set_grade!(topology, "i2", Online).unwrap(),
- peer!(2, "i2", "r2", "nowhere", Online),
+ set_grade!(topology, "i2", Grade::Online).unwrap(),
+ peer!(2, "i2", "r2", "nowhere", Grade::Online),
);
// idempotency
assert_eq!(
- set_grade!(topology, "i2", Online).unwrap(),
- peer!(2, "i2", "r2", "nowhere", Online),
+ set_grade!(topology, "i2", Grade::Online).unwrap(),
+ peer!(2, "i2", "r2", "nowhere", Grade::Online),
);
}
@@ -629,7 +638,7 @@ mod tests {
assert_eq!(peer.replicaset_id, "r1");
assert_eq!(
- set_grade!(t, "i3", Offline).unwrap().failure_domain,
+ set_grade!(t, "i3", Grade::Offline).unwrap().failure_domain,
faildoms! {planet: B, owner: V, dimension: C137},
);
diff --git a/test/int/test_couple.py b/test/int/test_couple.py
index c6bbabfa3f..d540b4b01c 100644
--- a/test/int/test_couple.py
+++ b/test/int/test_couple.py
@@ -174,7 +174,7 @@ def test_deactivation(cluster2: Cluster):
".raft_update_peer",
target.instance_id,
target.cluster_id,
- [{"Grade": grade}],
+ [{"Grade": grade}, {"TargetGrade": "Online"}],
)
# check idempotency
diff --git a/test/int/test_joining.py b/test/int/test_joining.py
index 59833f7b6b..475c0265be 100644
--- a/test/int/test_joining.py
+++ b/test/int/test_joining.py
@@ -152,6 +152,7 @@ def test_replication(cluster: Cluster):
instance.eval("return box.info.cluster.uuid"),
None,
"Online",
+ "Online",
dict(),
]
--
GitLab