Merge pull request sorintlab#510 from sgotti/keeper_report_error_when_local_dbuid_is_not_correct

keeper: report error when local dbuid is not correct

Author: sgotti

cmd/keeper/cmd/keeper.go

@@ -1052,40 +1052,15 @@ func (p *PostgresKeeper) postgresKeeperSM(pctx context.Context) {
 		}
 	}
 
-	initialized, err := pgm.IsInitialized()
-	if err != nil {
-		log.Errorw("failed to detect if instance is initialized", zap.Error(err))
-		return
-	}
-
-	if initialized {
-		var started bool
-		started, err = pgm.IsStarted()
+	if p.dbLocalState.UID != db.UID {
+		var initialized bool
+		initialized, err = pgm.IsInitialized()
 		if err != nil {
-			// log error getting instance state but go ahead.
-			log.Errorw("failed to retrieve instance status", zap.Error(err))
-		}
-		log.Debugw("db status", "initialized", true, "started", started)
-	} else {
-		log.Debugw("db status", "initialized", false, "started", false)
-	}
-
-	dbls = p.dbLocalStateCopy()
-	// if the db is initialized but there isn't a db local state then generate a new one
-	if initialized && dbls.UID == "" {
-		ndbls := &DBLocalState{
-			UID:          common.UID(),
-			Generation:   cluster.NoGeneration,
-			Initializing: false,
-		}
-		if err = p.saveDBLocalState(ndbls); err != nil {
-			log.Errorw("failed to save db local state", zap.Error(err))
+			log.Errorw("failed to detect if instance is initialized", zap.Error(err))
 			return
 		}
-	}
+		log.Infow("current db UID different than cluster data db UID", "db", p.dbLocalState.UID, "cdDB", db.UID)
 
-	if dbls.UID != db.UID {
-		log.Infow("current db UID different than cluster data db UID", "db", dbls.UID, "cdDB", db.UID)
 		switch db.Spec.InitMode {
 		case cluster.DBInitModeNew:
 			log.Infow("initializing the database cluster")
@@ -1125,7 +1100,6 @@ func (p *PostgresKeeper) postgresKeeperSM(pctx context.Context) {
 				log.Errorw("failed to initialize postgres database cluster", zap.Error(err))
 				return
 			}
-			initialized = true
 
 			if err = pgm.StartTmpMerged(); err != nil {
 				log.Errorw("failed to start instance", zap.Error(err))
@@ -1226,7 +1200,6 @@ func (p *PostgresKeeper) postgresKeeperSM(pctx context.Context) {
 					return
 				}
 			}
-			initialized = true
 
 			if err = pgm.StopIfStarted(true); err != nil {
 				log.Errorw("failed to stop pg instance", zap.Error(err))
@@ -1335,7 +1308,6 @@ func (p *PostgresKeeper) postgresKeeperSM(pctx context.Context) {
 					}
 				}
 			}
-			initialized = true
 
 		case cluster.DBInitModeExisting:
 			ndbls := &DBLocalState{
@@ -1384,24 +1356,32 @@ func (p *PostgresKeeper) postgresKeeperSM(pctx context.Context) {
 				return
 			}
 		case cluster.DBInitModeNone:
-			ndbls := &DBLocalState{
-				// replace our current db uid with the required one.
-				UID: db.UID,
-				// Set a no generation since we aren't already converged.
-				Generation:   cluster.NoGeneration,
-				Initializing: false,
-			}
-			if err = p.saveDBLocalState(ndbls); err != nil {
-				log.Errorw("failed to save db local state", zap.Error(err))
-				return
-			}
+			log.Errorw("different local dbUID but init mode is none, this shouldn't happen. Something bad happened to the keeper data. Check that keeper data is on a persistent volume and that the keeper state files weren't removed")
 			return
 		default:
 			log.Errorw("unknown db init mode", "initMode", string(db.Spec.InitMode))
 			return
 		}
 	}
 
+	initialized, err := pgm.IsInitialized()
+	if err != nil {
+		log.Errorw("failed to detect if instance is initialized", zap.Error(err))
+		return
+	}
+
+	if initialized {
+		var started bool
+		started, err = pgm.IsStarted()
+		if err != nil {
+			// log error getting instance state but go ahead.
+			log.Errorw("failed to retrieve instance status", zap.Error(err))
+		}
+		log.Debugw("db status", "initialized", true, "started", started)
+	} else {
+		log.Debugw("db status", "initialized", false, "started", false)
+	}
+
 	// create postgres parameteres
 	pgParameters = p.createPGParameters(db)
 	// update pgm postgres parameters

tests/integration/ha_test.go

@@ -1710,3 +1710,86 @@ func TestStandbyCantSync(t *testing.T) {
 		t.Fatalf("expected different dbuid for standbys[0]: got the same: %q", newStandby0DBUID)
 	}
 }
+
+// TestDisappearedKeeperData tests that, if keeper data disappears (at least
+// dbstate file is missing) and there's not init mode defined in the db spec, it'll
+// return en error
+func TestDisappearedKeeperData(t *testing.T) {
+	t.Parallel()
+
+	dir, err := ioutil.TempDir("", "stolon")
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	clusterName := uuid.NewV4().String()
+
+	tks, tss, tp, tstore := setupServers(t, clusterName, dir, 2, 1, false, false, nil)
+	defer shutdown(tks, tss, tp, tstore)
+
+	storePath := filepath.Join(common.StorePrefix, clusterName)
+	sm := store.NewKVBackedStore(tstore.store, storePath)
+
+	master, standbys := waitMasterStandbysReady(t, sm, tks)
+	standby := standbys[0]
+
+	if err := populate(t, master); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := write(t, master, 1, 1); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// get the master XLogPos
+	xLogPos, err := GetXLogPos(master)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	// wait for the keepers to have reported their state
+	if err := WaitClusterSyncedXLogPos([]*TestKeeper{master, standby}, xLogPos, sm, 20*time.Second); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// the proxy should connect to the right master
+	if err := tp.WaitRightMaster(master, 3*cluster.DefaultProxyCheckInterval); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// Stop the master keeper
+	t.Logf("Stopping current master keeper: %s", master.uid)
+	master.Stop()
+
+	// Remove master data
+	if err := os.RemoveAll(master.dataDir); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// restart master
+	t.Logf("Starting current master keeper: %s", master.uid)
+	master.Start()
+	waitKeeperReady(t, sm, master)
+
+	// master shouldn't start its postgres instance and standby should be elected as new master
+
+	// Wait for cluster data containing standby as master
+	if err := WaitClusterDataMaster(standby.uid, sm, 30*time.Second); err != nil {
+		t.Fatalf("expected master %q in cluster view", standby.uid)
+	}
+	if err := standby.WaitDBRole(common.RoleMaster, nil, 30*time.Second); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	c, err := getLines(t, standby)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if c != 1 {
+		t.Fatalf("wrong number of lines, want: %d, got: %d", 1, c)
+	}
+
+	// the proxy should connect to the right master
+	if err := tp.WaitRightMaster(standby, 3*cluster.DefaultProxyCheckInterval); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+}