Don't request indirect ping to same addr

If a member changed its identity while being probed there was
a chance foca would try to ask the new identity to ping
the old one on its behalf

Id: bb9ccbbb8ccd9ac171685a77377b4ce3835f9f6b
Author: Caio
Commit time: 2024-04-24T08:04:10+02:00

Modified CHANGELOG.md

@@ -1,5 +1,13
# Changelog

+## UNRELEASED
+
+- Bugfix: when restarting members, there was a chance foca would
+ ask a member to ping its own previous identity.
+ No harm done to the cluster state, but would lead to noise in
+ the form of `Error::DataFromOurselves` in the member logs
+ See: https://github.com/caio/foca/issues/34
+
## v0.17.0 - 2024-03-20

This release contains significant changes aimed at freeing users

Modified src/lib.rs

@@ -646,6 +646,17
return Ok(());
}

+ if !self.members.is_active(&probed_id) {
+ // Probed member is not active anymore
+ // Nothing else to be done this probe cycle
+ #[cfg(feature = "tracing")]
+ tracing::debug!(
+ probed_id = tracing::field::debug(&probed_id),
+ "Probed member isn't active anymore"
+ );
+ return Ok(());
+ }
+
self.member_buf.clear();
self.members.choose_active_members(
self.config.num_indirect_probes.get(),

@@ -4245,5 +4256,33
assert_eq!(1, foca.num_members());
assert!(runtime.is_empty());
}
+ }
+
+ #[test]
+ fn no_indirect_cycle_if_probed_disappears() {
+ // ref https://github.com/caio/foca/issues/34
+
+ let (mut foca, probed, send_indirect_probe) = craft_probing_foca(1, config());
+ let mut runtime = AccumulatingRuntime::new();
+ assert_eq!(1, foca.num_members());
+
+ // while `foca` is probing `probed`, it learns
+ // that it changed its identity for whatever reason
+ let renewed = probed.bump();
+ assert_eq!(Ok(()), foca.apply(Member::alive(renewed), &mut runtime));
+ assert_eq!(1, foca.num_members());
+ runtime.clear();
+
+ // So when the indirect part of the cycle fires
+ assert_eq!(Ok(()), foca.handle_timer(send_indirect_probe, &mut runtime));
+
+ // Since `renewed` and `probed` are technically the same
+ assert_eq!(renewed.addr(), probed.addr());
+ assert!(renewed.win_addr_conflict(&probed));
+ assert!(!probed.win_addr_conflict(&renewed));
+
+ // Foca must not request `renewed` to probe `probed`
+ // on its behalf
+ assert!(runtime.take_all_data().is_empty());
}
}

Modified src/member.rs

@@ -276,6 +276,12
self.inner.iter().filter(|m| m.is_active())
}

+ pub(crate) fn is_active(&self, id: &T) -> bool {
+ self.inner
+ .iter()
+ .any(|member| &member.id == id && member.is_active())
+ }
+
pub(crate) fn apply_existing_if<F: Fn(&Member<T>) -> bool>(
&mut self,
mut update: Member<T>,