Multihoming: Hotswap gossip socketaddr

[gregcusack]

Problem

No ability to swap IP address for multihoming support

Summary of Changes

1. Add AdminRPC command to rebind gossip socket and refresh gossip contactinfo
2. Add channel to read command and execute hot swap of gossip socket
3. Update streamer to take in an AtomicUdpSocket which is hot swapable.

HOW TO:

• Say you want to switch your gossip port from 8000 to 9998. Run:

echo '{"jsonrpc":"2.0","id":1,"method":"setGossipSocket","params":["<your-nodes-ip>",9998]}' | socat - ./<path-to-admin-rpc>

example:

echo '{"jsonrpc":"2.0","id":1,"method":"setGossipSocket","params":["xxx.xxx.xx.x",9998]}' | socat - ./admin.rpc

[codecov-commenter]

Codecov Report

❌ Patch coverage is 72.22222% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.4%. Comparing base (bbea86a) to head (1809a7a).
⚠️ Report is 3084 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #6474     +/-   ##
=========================================
- Coverage    83.4%    83.4%   -0.1%     
=========================================
  Files         850      851      +1     
  Lines      377710   377863    +153     
=========================================
+ Hits       315086   315166     +80     
- Misses      62624    62697     +73     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alexpyattaev]

Overall, I think this is a good approach for now, we should follow through. I'll clean up the socket binding logic to make it easier to integrate this for other services with more esoteric binding patterns.

[gregcusack]

in my last commit I took out last_id which was needed to determine if the socket was set, even for CurrentSocket::Same(). needed this to set the read timeout in recv_loop() when first called. ended up putting that logic in recv_loop() instead of putting it back in SocketProvider

[alexpyattaev]

I wonder, since we are sending a socket and a rebinder to the service, would it not be better if we sent both in the same argument? They are logically tied to each other fairly hard... Would it make sense to send AtomicUdpSocket rather than a UdpSocket and a notification channel separately?

[gregcusack]

so we create GossipService in bootstrap as well, but we don't necessarily want the GossipService in bootstrap to be rebindable: https://github.com/gregcusack/solana/blob/2e4203abd39239a8fce1ff7894bdb28f354f9c1b/validator/src/bootstrap.rs#L161. We could but it will never be used.

we could create some GossipSocket enum like:

pub enum GossipSocket {
    Static(UdpSocket)
    Rebindable(RebindableSocket) // maybe not the best name lol
}

where

pub struct RebindableSocket {
    pub socket: Arc<AtomicUdpSocket>,
    pub rebind_rx: Receiver<SocketAddr>,
}

[alexpyattaev]

Maybe use an equivalent of RebindableSocket always and just fill rebind_rx with https://docs.rs/crossbeam/latest/crossbeam/channel/fn.never.html ? Then you can trivially create one from UdpSocket for cases when rebinding is not necessary.

[gregcusack]

ended up just removing the crossbeam channel altogether

[alexpyattaev]

Given that binding a socket takes literally microseconds, do we need a thread waiting on a channel to do it? Can we maybe rebind immediately and just swap the thing in place immediately in the admin RPC handling code?

[alexpyattaev]

Do you think it would be possible to perform the actual rebinding here and avoid having the channel and Rebinder abstraction on top of it? We are not calling this every second, so even if this RPC blocks for a few milliseconds it is not a big deal.

[gregcusack]

much better idea. yes i can do that

[alexpyattaev]

LGTM, this does not need to be perfect at the moment.

[gregcusack]

LGTM, this does not need to be perfect at the moment.

I'd like to wait on merging this until I can test on a box w/ two routable IPs just to make sure we're good to go here

[gregcusack]

pushed another commit: 1809a7a

removes a double Arc around the hotswapable socket and changes Sockets struct to hold an AtomicUdpSocket for gossip socket

[gregcusack]

@alexpyattaev UPDATE: this has been tested on a multihomed machine and is working! Now that we have support to pass in --bind-address, I wonder if it makes sense to close up the AdminRPC Api on this?

Probably doesn't really matter. But currently you can pass in any IP/port into AdminRPC. Of course, if you pass in an IP that doesn't exist as an interface on your machine the swap will fail. Should we instead check if the IP exists in BindIpAddrs first? Run the swap if it does. If not, log and return an error?

[alexpyattaev]

Let us not add this coupling just yet. Keeping the changeset small has benefits too. For now we can just assume the adminRPC does not get garbage in (which should be a reasonable assumption for what is an experimental feature at the moment). Once we have the logic mostly sorted we can add idiotproofing where appropriate.