Skip to content
Snippets Groups Projects
Commit a06ff88d authored by Yaroslav Dynnikov's avatar Yaroslav Dynnikov
Browse files

fix: remove unique index on peer_address

The `peer_address` parameter is an inbound address used for
communication with the peer. It shouldn't be confused with the listen
address. The persisted `peer_address` may become obsolete due to
circumstances beyond picodata control (e.g. DNS or IP changes). Thus
there's no point in its prior validation, including the uniqueness
check.

There's also no such task as getting peer by peer_address.

To sum up, an index over `peer_address` is useless. It only creates
problems and causes panics.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/88
parent 08f68542
No related branches found
No related tags found
1 merge request!123fix: remove unique index on peer_address
Pipeline #5408 passed
...@@ -65,8 +65,14 @@ pub enum Op { ...@@ -65,8 +65,14 @@ pub enum Op {
/// Serializable struct representing a member of the raft group. /// Serializable struct representing a member of the raft group.
#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)]
pub struct Peer { pub struct Peer {
/// Used for identifying raft nodes.
/// Must be unique in the raft group.
pub raft_id: u64, pub raft_id: u64,
/// Inbound address used for communication with the node.
/// Not to be confused with listen address.
pub peer_address: String, pub peer_address: String,
/// Reflects the role of the node in the raft group.
/// Non-voters are also called learners in terms of raft.
pub voter: bool, pub voter: bool,
pub instance_id: String, pub instance_id: String,
pub replicaset_id: String, pub replicaset_id: String,
......
...@@ -97,11 +97,6 @@ impl Storage { ...@@ -97,11 +97,6 @@ impl Storage {
parts = {{'replicaset_id'}, {'commit_index'}}, parts = {{'replicaset_id'}, {'commit_index'}},
unique = false, unique = false,
}) })
box.space.raft_group:create_index('peer_address', {
if_not_exists = true,
parts = {{'peer_address'}},
unique = true,
})
"#, "#,
); );
} }
...@@ -558,24 +553,19 @@ inventory::submit!(crate::InnerTest { ...@@ -558,24 +553,19 @@ inventory::submit!(crate::InnerTest {
) )
); );
assert_err!( {
Storage::persist_peer(&traft::Peer { // Ensure traft storage doesn't impose restrictions
raft_id: 99, // on peer_address uniqueness.
peer_address: "addr:1".into(), let peer = |id: u64, addr: &str| traft::Peer {
raft_id: id,
instance_id: format!("i{id}"),
peer_address: addr.into(),
..Default::default() ..Default::default()
}), };
concat!(
"unknown error", Storage::persist_peer(&peer(10, "addr:collision")).unwrap();
" Tarantool error:", Storage::persist_peer(&peer(11, "addr:collision")).unwrap();
" TupleFound: Duplicate key exists", }
" in unique index \"peer_address\"",
" in space \"raft_group\"",
" with old tuple",
" - [1, \"addr:1\", true, \"i1\", \"r1\", \"i1-uuid\", \"r1-uuid\", 1]",
" and new tuple",
" - [99, \"addr:1\", false, \"\", \"\", \"\", \"\", 0]"
)
);
let peer_by_raft_id = |id: u64| Storage::peer_by_raft_id(id).unwrap().unwrap(); let peer_by_raft_id = |id: u64| Storage::peer_by_raft_id(id).unwrap().unwrap();
{ {
...@@ -594,6 +584,10 @@ inventory::submit!(crate::InnerTest { ...@@ -594,6 +584,10 @@ inventory::submit!(crate::InnerTest {
assert_eq!(peer_by_instance_id("i3").peer_address, "addr:3"); assert_eq!(peer_by_instance_id("i3").peer_address, "addr:3");
assert_eq!(peer_by_instance_id("i4").peer_address, "addr:4"); assert_eq!(peer_by_instance_id("i4").peer_address, "addr:4");
assert_eq!(peer_by_instance_id("i5").peer_address, "addr:5"); assert_eq!(peer_by_instance_id("i5").peer_address, "addr:5");
assert_eq!(
peer_by_instance_id("i10").peer_address,
peer_by_instance_id("i11").peer_address
);
assert_eq!(Storage::peer_by_instance_id("i6"), Ok(None)); assert_eq!(Storage::peer_by_instance_id("i6"), Ok(None));
} }
...@@ -642,7 +636,6 @@ inventory::submit!(crate::InnerTest { ...@@ -642,7 +636,6 @@ inventory::submit!(crate::InnerTest {
) )
); );
raft_group.index("peer_address").unwrap().drop().unwrap();
raft_group.primary_key().drop().unwrap(); raft_group.primary_key().drop().unwrap();
assert_err!( assert_err!(
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment