core: call dht bootstrap

core/bootstrap.go

@@ -31,6 +31,8 @@ func superviseConnections(parent context.Context,
 	store peer.Peerstore,
 	peers []peer.PeerInfo) error {
 
+	var dhtAlreadyBootstrapping bool
+
 	for {
 		ctx, _ := context.WithTimeout(parent, connectiontimeout)
 		// TODO get config from disk so |peers| always reflects the latest
@@ -38,6 +40,14 @@ func superviseConnections(parent context.Context,
 		if err := bootstrap(ctx, h, route, store, peers); err != nil {
 			log.Error(err)
 		}
+
+		if !dhtAlreadyBootstrapping {
+			dhtAlreadyBootstrapping = true // only call dht.Bootstrap once.
+			if _, err := route.Bootstrap(); err != nil {
+				log.Error(err)
+			}
+		}
+
 		select {
 		case <-parent.Done():
 			return parent.Err()
@@ -56,7 +66,7 @@ func bootstrap(ctx context.Context,
 	connectedPeers := h.Network().Peers()
 	if len(connectedPeers) >= recoveryThreshold {
 		log.Event(ctx, "bootstrapSkip", h.ID())
-		log.Debugf("%s bootstrap skipped -- connected to %d (> %d) nodes",
+		log.Debugf("%s core bootstrap skipped -- connected to %d (> %d) nodes",
 			h.ID(), len(connectedPeers), recoveryThreshold)
 
 		return nil
@@ -64,7 +74,7 @@ func bootstrap(ctx context.Context,
 	numCxnsToCreate := recoveryThreshold - len(connectedPeers)
 
 	log.Event(ctx, "bootstrapStart", h.ID())
-	log.Debugf("%s bootstrapping to %d more nodes", h.ID(), numCxnsToCreate)
+	log.Debugf("%s core bootstrapping to %d more nodes", h.ID(), numCxnsToCreate)
 
 	var notConnected []peer.PeerInfo
 	for _, p := range bootstrapPeers {
@@ -83,15 +93,6 @@ func bootstrap(ctx context.Context,
 			return err
 		}
 	}
-
-	// we can try running dht bootstrap even if we're connected to all bootstrap peers.
-	if len(h.Network().Conns()) > 0 {
-		if _, err := r.Bootstrap(); err != nil {
-			// log this as Info. later on, discern better between errors.
-			log.Infof("dht bootstrap err: %s", err)
-			return nil
-		}
-	}
 	return nil
 }
 

core/core.go

@@ -235,29 +235,17 @@ func (n *IpfsNode) StartOnlineServices(ctx context.Context) error {
 	// TODO implement an offline namesys that serves only local names.
 	n.Namesys = namesys.NewNameSystem(n.Routing)
 
-	// TODO consider moving connection supervision into the Network. We've
-	// discussed improvements to this Node constructor. One improvement
-	// would be to make the node configurable, allowing clients to inject
-	// an Exchange, Network, or Routing component and have the constructor
-	// manage the wiring. In that scenario, this dangling function is a bit
-	// awkward.
-	var bootstrapPeers []peer.PeerInfo
-	for _, bootstrap := range n.Repo.Config().Bootstrap {
-		p, err := toPeer(bootstrap)
-		if err != nil {
-			log.Event(ctx, "bootstrapError", n.Identity, lgbl.Error(err))
-			log.Errorf("%s bootstrap error: %s", n.Identity, err)
-			return err
-		}
-		bootstrapPeers = append(bootstrapPeers, p)
-	}
-
-	go superviseConnections(ctx, n.PeerHost, dhtRouting, n.Peerstore, bootstrapPeers)
-
 	n.Reprovider = rp.NewReprovider(n.Routing, n.Blockstore)
 	go n.Reprovider.ProvideEvery(ctx, kReprovideFrequency)
 
-	return nil
+	// prepare bootstrap peers from config
+	bpeers, err := n.loadBootstrapPeers()
+	if err != nil {
+		log.Event(ctx, "bootstrapError", n.Identity, lgbl.Error(err))
+		log.Errorf("%s bootstrap error: %s", n.Identity, err)
+		return debugerror.Wrap(err)
+	}
+	return n.Bootstrap(ctx, bpeers)
 }
 
 // teardown closes owned children. If any errors occur, this function returns
@@ -310,11 +298,28 @@ func (n *IpfsNode) Bootstrap(ctx context.Context, peers []peer.PeerInfo) error {
 
 	// TODO what should return value be when in offlineMode?
 
-	if n.Routing != nil {
-		if dht, ok := n.Routing.(*dht.IpfsDHT); ok {
-			return bootstrap(ctx, n.PeerHost, dht, n.Peerstore, peers)
-		}
+	if n.Routing == nil {
+		return nil
+	}
+
+	// TODO what bootstrapping should happen if there is no DHT? i.e. we could
+	// continue connecting to our bootstrap peers, but for what purpose?
+	dhtRouting, ok := n.Routing.(*dht.IpfsDHT)
+	if !ok {
+		return nil
 	}
+
+	// TODO consider moving connection supervision into the Network. We've
+	// discussed improvements to this Node constructor. One improvement
+	// would be to make the node configurable, allowing clients to inject
+	// an Exchange, Network, or Routing component and have the constructor
+	// manage the wiring. In that scenario, this dangling function is a bit
+	// awkward.
+
+	// spin off the node's connection supervisor.
+	// TODO, clean up how this thing works. Make the superviseConnections thing
+	// work like the DHT.Bootstrap.
+	go superviseConnections(ctx, n.PeerHost, dhtRouting, n.Peerstore, peers)
 	return nil
 }
 
@@ -355,6 +360,18 @@ func (n *IpfsNode) loadPrivateKey() error {
 	return nil
 }
 
+func (n *IpfsNode) loadBootstrapPeers() ([]peer.PeerInfo, error) {
+	var peers []peer.PeerInfo
+	for _, bootstrap := range n.Repo.Config().Bootstrap {
+		p, err := toPeer(bootstrap)
+		if err != nil {
+			return nil, err
+		}
+		peers = append(peers, p)
+	}
+	return peers, nil
+}
+
 // SetupOfflineRouting loads the local nodes private key and
 // uses it to instantiate a routing system in offline mode.
 // This is primarily used for offline ipns modifications.