ruserf icon indicating copy to clipboard operation
ruserf copied to clipboard

Suggest changing `join` and `join_many` parameters from `Node` to pure `Address`

Open imlk0 opened this issue 1 month ago • 1 comments

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.

imlk0 avatar Nov 27 '25 02:11 imlk0

Hi, thanks for pointing it out! I would like to make such changes.

al8n avatar Nov 27 '25 02:11 al8n