server: start jobs before running migrations

Migrations sometimes try to run jobs, for example running schema changes or other operations that are executed as jobs. Thus the jobs system must be started before the migrations are run. Release note: none.
cockroachdb · Mar 7, 2020 · 2cefc00 · 2cefc00
1 parent c5e8099
commit 2cefc00
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 34 deletions.
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -1700,23 +1700,6 @@ func (s *Server) Start(ctx context.Context) error {
 		s.ClusterSettings(),
 	)
 
-	var bootstrapVersion roachpb.Version
-	if err := s.db.Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
-		return txn.GetProto(ctx, keys.BootstrapVersionKey, &bootstrapVersion)
-	}); err != nil {
-		return err
-	}
-	if err := migMgr.EnsureMigrations(ctx, bootstrapVersion); err != nil {
-		select {
-		case <-s.stopper.ShouldQuiesce():
-			// Avoid turning an early shutdown into a fatal error. See #19579.
-			return errors.New("server is shutting down")
-		default:
-			log.Fatalf(ctx, "%+v", err)
-		}
-	}
-	log.Infof(ctx, "done ensuring all necessary migrations have run")
-
 	// Start garbage collecting system events.
 	s.startSystemLogsGC(ctx)
 
@@ -1758,6 +1741,37 @@ func (s *Server) Start(ctx context.Context) error {
 	s.mux.Handle(statusVars, http.HandlerFunc(s.status.handleVars))
 	log.Event(ctx, "added http endpoints")
 
+	// Start the jobs subsystem.
+	{
+		var regLiveness jobs.NodeLiveness = s.nodeLiveness
+		if testingLiveness := s.cfg.TestingKnobs.RegistryLiveness; testingLiveness != nil {
+			regLiveness = testingLiveness.(*jobs.FakeNodeLiveness)
+		}
+		if err := s.jobRegistry.Start(
+			ctx, s.stopper, regLiveness, jobs.DefaultCancelInterval, jobs.DefaultAdoptInterval,
+		); err != nil {
+			return err
+		}
+	}
+
+	// Run startup migrations (note: these depend on jobs subsystem running).
+	var bootstrapVersion roachpb.Version
+	if err := s.db.Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
+		return txn.GetProto(ctx, keys.BootstrapVersionKey, &bootstrapVersion)
+	}); err != nil {
+		return err
+	}
+	if err := migMgr.EnsureMigrations(ctx, bootstrapVersion); err != nil {
+		select {
+		case <-s.stopper.ShouldQuiesce():
+			// Avoid turning an early shutdown into a fatal error. See #19579.
+			return errors.New("server is shutting down")
+		default:
+			log.Fatalf(ctx, "%+v", err)
+		}
+	}
+	log.Infof(ctx, "done ensuring all necessary migrations have run")
+
 	// Attempt to upgrade cluster version.
 	s.startAttemptUpgrade(ctx)
 
@@ -1794,18 +1808,6 @@ func (s *Server) Start(ctx context.Context) error {
 	// node. This also uses SQL.
 	s.leaseMgr.DeleteOrphanedLeases(timeThreshold)
 
-	{
-		var regLiveness jobs.NodeLiveness = s.nodeLiveness
-		if testingLiveness := s.cfg.TestingKnobs.RegistryLiveness; testingLiveness != nil {
-			regLiveness = testingLiveness.(*jobs.FakeNodeLiveness)
-		}
-		if err := s.jobRegistry.Start(
-			ctx, s.stopper, regLiveness, jobs.DefaultCancelInterval, jobs.DefaultAdoptInterval,
-		); err != nil {
-			return err
-		}
-	}
-
 	log.Event(ctx, "server ready")
 
 	return nil

diff --git a/pkg/sql/sqlbase/system.go b/pkg/sql/sqlbase/system.go
@@ -149,7 +149,7 @@ CREATE TABLE system.jobs (
 	status            STRING    NOT NULL,
 	created           TIMESTAMP NOT NULL DEFAULT now(),
 	payload           BYTES     NOT NULL,
-	payload           BYTES,
+	progress          BYTES,
 	INDEX (status, created),
 	FAMILY (id, status, created, payload),
 	FAMILY progress (progress)
@@ -730,10 +730,11 @@ var (
 				ColumnIDs:   []ColumnID{1, 2, 3, 4},
 			},
 			{
-				Name:        "fam_1_progress",
-				ID:          1,
-				ColumnNames: []string{"progress"},
-				ColumnIDs:   []ColumnID{5},
+				Name:            "progress",
+				ID:              1,
+				ColumnNames:     []string{"progress"},
+				ColumnIDs:       []ColumnID{5},
+				DefaultColumnID: 5,
 			},
 		},
 		NextFamilyID: 2,