server: defer gossip start to when absolutely needed

This was pulled out of #52526 to keep the diff there focussed on the
introduction of the RPC (and to see if that alone shaked out any
failures). That change lets us make this one, were we can defer gossip
start until right before we start up Node.

Release justification: low risk, high benefit change to existing functionality
Release note: None

pkg/server/node.go

@@ -467,12 +467,13 @@ func (n *Node) start(
 
 	n.startComputePeriodicMetrics(n.stopper, base.DefaultMetricsSampleInterval)
 
-	// Be careful about moving this line above `startStores`; store migrations rely
-	// on the fact that the cluster version has not been updated via Gossip (we
-	// have migrations that want to run only if the server starts with a given
-	// cluster version, but not if the server starts with a lower one and gets
-	// bumped immediately, which would be possible if gossip got started earlier).
-	n.startGossip(ctx, n.stopper)
+	// Be careful about moving this line above where we start stores; store
+	// migrations rely on the fact that the cluster version has not been updated
+	// via Gossip (we have migrations that want to run only if the server starts
+	// with a given cluster version, but not if the server starts with a lower
+	// one and gets bumped immediately, which would be possible if gossip got
+	// started earlier).
+	n.startGossipping(ctx, n.stopper)
 
 	allEngines := append([]storage.Engine(nil), state.initializedEngines...)
 	allEngines = append(allEngines, state.newEngines...)
@@ -592,7 +593,7 @@ func (n *Node) bootstrapStores(
 			}
 			n.addStore(ctx, s)
 			log.Infof(ctx, "bootstrapped store %s", s)
-			// Done regularly in Node.startGossip, but this cuts down the time
+			// Done regularly in Node.startGossipping, but this cuts down the time
 			// until this store is used for range allocations.
 			if err := s.GossipStore(ctx, false /* useCached */); err != nil {
 				log.Warningf(ctx, "error doing initial gossiping: %s", err)
@@ -611,9 +612,9 @@ func (n *Node) bootstrapStores(
 	return nil
 }
 
-// startGossip loops on a periodic ticker to gossip node-related
+// startGossipping loops on a periodic ticker to gossip node-related
 // information. Starts a goroutine to loop until the node is closed.
-func (n *Node) startGossip(ctx context.Context, stopper *stop.Stopper) {
+func (n *Node) startGossipping(ctx context.Context, stopper *stop.Stopper) {
 	ctx = n.AnnotateCtx(ctx)
 	stopper.RunWorker(ctx, func(ctx context.Context) {
 		// Verify we've already gossiped our node descriptor.

pkg/server/server.go

@@ -1302,8 +1302,6 @@ func (s *Server) Start(ctx context.Context) error {
 		}
 	}
 
-	// TODO(irfansharif): How late can we defer gossip start to?
-	startGossipFn()
 	if s.cfg.DelayedBootstrapFn != nil {
 		defer time.AfterFunc(30*time.Second, s.cfg.DelayedBootstrapFn).Stop()
 	}
@@ -1466,6 +1464,9 @@ func (s *Server) Start(ctx context.Context) error {
 
 	onSuccessfulReturnFn()
 
+	// We're going to need to start gossip before we spin up Node below.
+	startGossipFn()
+
 	// Now that we have a monotonic HLC wrt previous incarnations of the process,
 	// init all the replicas. At this point *some* store has been bootstrapped or
 	// we're joining an existing cluster for the first time.