ruserf
ruserf copied to clipboard
Suggest changing `join` and `join_many` parameters from `Node` to pure `Address`
Hi, I'd like to suggest an improvement to the join and join_many methods in the Serf API.
Currently, both functions require full Node<T::Id, ...> instances as input:
pub async fn join(
&self,
node: Node<T::Id, MaybeResolvedAddress<T::Address, T::ResolvedAddress>>,
ignore_old: bool,
) -> Result<Node<T::Id, T::ResolvedAddress>, Error<T, D>>
pub async fn join_many(
&self,
existing: impl Iterator<Item = Node<T::Id, MaybeResolvedAddress<T::Address, T::ResolvedAddress>>>,
ignore_old: bool,
) -> Result<
SmallVec<Node<T::Id, T::ResolvedAddress>>,
(SmallVec<Node<T::Id, T::ResolvedAddress>>, Error<T, D>),
>;
But since the NodeId is ignored during the join process, requiring it forces users to either:
- Make up a dummy NodeId (e.g., zero or random),
- Or hardcode one they don’t know — leading to confusion and misuse.
This is especially problematic because:
- Users joining a cluster typically only know the addresses of existing members (IP:Port), not their NodeIds.
- This behavior diverges from HashiCorp's original Go serf implementation, where Join accepts only a list of strings (addresses), with no need for node identity.
Proposal
Update both join and join_many to accept plain addresses instead of full Node values:
pub async fn join(
&self,
address: MaybeResolvedAddress<T::Address, T::ResolvedAddress>,
ignore_old: bool,
) -> Result<Node<T::Id, T::ResolvedAddress>, Error<T, D>>
pub async fn join_many<A: Into<T::Address> + Clone>(
&self,
existing: impl Iterator<Item = MaybeResolvedAddress<T::Address, T::ResolvedAddress>>,
ignore_old: bool,
) -> Result<
SmallVec<T::ResolvedAddress>,
(SmallVec<T::ResolvedAddress>, Error<T, D>),
>
The Serf implementation can then internally construct temporary contact points using only the provided addresses, and discover the actual NodeIds during the join process — which is how Serf is designed to work.
Hi, thanks for pointing it out! I would like to make such changes.