Run fixLowPeers on startup

[samsondav]

The git diff makes this look more invasive than it really is.

All I did was pull some logic out of fixLowPeersRoutine into a separate function and make it public. None of the functionality has been changed.

[aschmahmann]

@samsondav this is exposing a fairly internal function that is subject to change. What are you hoping to use it for? Are you just trying to make sure that it's called at startup, if so there are some other options available to us including:

1. Just making it always run on startup
2. Having an option to make it run on startup
3. Have Bootstrap call fixRTIfNeeded

[samsondav]

We need it to run on startup, since we immediately query the DHT after instantiating it and we get "no peers found" error even though we provided bootstrap nodes.

[samsondav]

What do you think the best way to solve this problem is?

[aschmahmann]

I'm not sure offhand which of these three options is the best, although I suspect that having it always run on startup makes the most sense.

@petar sound like a plan?

[petar]

I would have chosen adding as part of Bootstrap. So that at API level there is a clear contract that routing operations work after invoking a Bootsrap.

[samsondav]

@petar @aschmahmann I added it to Bootstrap, what do you think?

[aschmahmann]

Seems good, just a couple comments to change and we're good to go. Thanks for the PR 😄

[samsondav]

@petar commented this:

FixLowPeers should be called and finish strictly before RefreshManager starts. We can hook this up.
Concurrent user calls to Bootstrap (which will also invoke FixLowPeers) should be ok.

(pasted from slack convo)

[petar]

fixLowPeers must be syncronized with refresh manager trigger

[samsondav]

@petar @aschmahmann I've updated it based on your most recent comments, would you mind taking another look?

[aschmahmann]

Thanks for doing this and apologies for the delayed review. Left a couple small change requests and a question.

[aschmahmann]

Let's not put this in a separate goprocess and leave it in the init function, it's nice to return the errors on init since if we fail to start the refresh manager the DHT is dead. A log error, isn't nearly as good.

[samsondav]

This is contrary to what @petar said above:

FixLowPeers should be called and finish strictly before RefreshManager starts

[aschmahmann]

Ah fair enough, for now this may to reduce start up errors from refreshing the routing table while it's still empty.

However, calling host.Connect() on a bootstrap peer will not actually add the peer to the routing table immediately as that's triggered by a separate event. If we want to reduce those errors we'll have to add something that actually waits on peers initially being added to the routing table.