caio.co/de/foca

Add docs to the new Identity trait methods

Id
0a5c036065c7df400cf712df5b7fe9333cd251d3
Author
Caio
Commit time
2024-03-17T20:55:11+01:00

Modified examples/foca_insecure_udp_agent.rs

@@ -156,7 +156,9
self.addr
}

- // FIXME explain
+ // This teaches every member how to compare two identities
+ // with the same Addr value
+ // In our case, the one with the larger bump always wins
fn win_addr_conflict(&self, adversary: &Self) -> bool {
self.bump > adversary.bump
}

Modified examples/identity_golf.rs

@@ -56,7 +56,12
self.addr
}

- // FIXME explain
+ // When an identity is renew()ed, the cluster will start
+ // seeing two distinct FatIdentity with the exact same
+ // Addr.
+ // This teaches foca to choose the right one to keep
+ // In this case, the right one is simply the one with
+ // the higher `extra` field
fn win_addr_conflict(&self, adversary: &Self) -> bool {
self.extra > adversary.extra
}

Modified src/identity.rs

@@ -27,7 +27,13
/// See `examples/identity_golf.rs` for ideas
///
pub trait Identity: Clone + Eq + fmt::Debug {
- /// FIXME docs
+ /// The type of the unique (cluster-wide) address of this identity
+ ///
+ /// A plain identity that cannot auto-rejoin (see `Identity::renew`)
+ /// could have `Addr` the same as `Self` (`std::net::SocketAddr` is
+ /// an example of one)
+ ///
+ /// It's a good idea to have this type as lean as possible
type Addr: PartialEq;

/// Opt-in on auto-rejoining by providing a new identity.
@@ -37,14 +43,25
/// identity and if it yields a new one will immediately
/// switch to it and notify the cluster so that downtime is
/// minimized.
+ ///
+ /// **NOTE** The new identity must win the conflict
fn renew(&self) -> Option<Self>;

- /// FIXME missing docs
+ /// Return this identity's unique address
+ ///
+ /// Typically a socket address, a hostname or similar
+ ///
+ /// On previous versions of this crate, there was a `has_same_prefix()`
+ /// method. This serves the same purpose. Having a concrete type
+ /// instead of just a yes/no allows Foca to fully manage the
+ /// cluster members and keep its memory bound by the number of nodes
+ /// instead of the number of identities
fn addr(&self) -> Self::Addr;

- /// FIXME docs
- // decide which identity to keep in case of conflicts
- // ffs this name
+ /// Decides which to keep when Foca encounters multiple identities
+ /// sharing the same address
+ ///
+ /// Returning `true` means that self will be kept
fn win_addr_conflict(&self, _adversary: &Self) -> bool;
}

Modified src/lib.rs

@@ -1581,6 +1581,14
#[cfg(feature = "tracing")]
tracing::debug!("Rejoin failure: Identity::renew() returned same id",);
Ok(false)
+ } else if !new_identity.win_addr_conflict(&self.identity) {
+ #[cfg(feature = "tracing")]
+ tracing::warn!(
+ new = tracing::field::debug(&new_identity),
+ old = tracing::field::debug(&self.identity),
+ "Rejoin failure: New identity doesn't win the conflict with the old one",
+ );
+ Ok(false)
} else {
self.change_identity(new_identity.clone(), &mut runtime)?;

@@ -1706,12 +1714,14

codec
.encode_header(&header, &mut buf)
- .expect("MAYBE FIXME?");
+ .expect("BadCodec shouldn't fail");

if !updates.is_empty() {
buf.put_u16(u16::try_from(updates.len()).unwrap());
for member in updates.iter() {
- codec.encode_member(member, &mut buf).expect("MAYBE FIXME?");
+ codec
+ .encode_member(member, &mut buf)
+ .expect("BadCodec shouldn't fail");
}
}

Modified src/probe.rs

@@ -6,8 +6,6

use crate::{member::Member, ProbeNumber};

-// FIXME This whole thing is ugly AF :(
-
pub(crate) struct Probe<T> {
direct: Option<Member<T>>,
indirect: Vec<T>,