search

jmsadair / raft

An implementation of the Raft consensus protocol.
MIT License
34 stars 3 forks source link

Add support for advertised addresses #85

Closed bubbajoe closed 2 months ago

bubbajoe commented 3 months ago

This implementation assumes that the address that is being listened on (e.g. 0.0.0.0:1234), is the same address that other nodes will use to connect. This is usually the case for testing locally, but in staging/production, it's common to use domain names, and/or different addresses.

I don't think there is much to change except removing the address comparison here

To get around this, I am manually adding Transport and using the advertised address as the address variable for the NewRaft function.

jmsadair commented 2 months ago

Apologies for the delay in getting to this. This seems simple enough - to be clear, you would like to be able to bootstrap a node with a configuration that contains a differing address than the address the node is initialized with?

For example, let's say that that we initialize a node using the NewRaft function with ID n and address 127.0.0.1. You would like the ability to call Bootstrap on this node with a configuration containing an ID-address pair such as (n, 127.0.0.2). Is that right?

bubbajoe commented 2 months ago

Yep, basically 2 different address concepts. Listen Address (which could be 0.0.0.0:1234) and Advertised Address (which could be node1.domain.top:4321).

Advertised Address should always default to the Listen Address. It would be nice to add this in the library but this would require breaking changes since you don't use configs.

jmsadair commented 2 months ago

Great. In the future, feel free to make pull requests yourself - you don't have to, but I occasionally get caught up with work or other stuff and it may take me a while to get to it. If it's a simple change like this and you put up a PR, I'll make sure to review it in a timely manner.