diff --git a/test/multi_master/chaos_test.go b/test/multi_master/chaos_test.go new file mode 100644 index 000000000..50a2be6ca --- /dev/null +++ b/test/multi_master/chaos_test.go @@ -0,0 +1,186 @@ +package multi_master + +import ( + "fmt" + "math/rand/v2" + "os" + "strconv" + "testing" + "time" +) + +// chaosRounds is one stop or start per round, enough to walk in and out of +// quorum several times without turning this into a soak test. +const chaosRounds = 12 + +// TestRandomStartStopElection bounces masters at random and holds the election +// to the two things it must never get wrong: two masters claiming leadership at +// once, and a quorum that comes back without agreeing on one. The cluster's +// identity has to survive the whole walk — a master that re-mints a TopologyId +// here is the split brain SetTopologyId kills its peers over. +func TestRandomStartStopElection(t *testing.T) { + for _, impl := range raftImplementations { + t.Run(impl.name, func(t *testing.T) { + seed := chaosSeed(t) + t.Logf("seed %d — replay this walk with MULTI_MASTER_IT_SEED=%d", seed, seed) + rng := rand.New(rand.NewPCG(seed, seed)) + + mc := NewMasterCluster(t, impl.raftHashicorp) + for i := range 3 { + mc.StartNode(i) + } + if err := mc.WaitForLeader(waitTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("cluster did not elect a leader: %v", err) + } + topologyId, err := mc.WaitForTopologyId(waitTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("no initial TopologyId: %v", err) + } + + for round := 1; round <= chaosRounds; round++ { + target := rng.IntN(3) + if mc.IsNodeRunning(target) { + mc.StopNode(target) + t.Logf("round %d: stopped master %d, %d left running", round, target, runningMasters(mc)) + } else { + mc.StartNode(target) + t.Logf("round %d: started master %d, %d now running", round, target, runningMasters(mc)) + } + + if err := waitForSettledElection(mc, leaderElectionTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("round %d (seed %d): %v", round, seed, err) + } + + // /dir/status proxies to the leader, so this reads the value the + // cluster as a whole is carrying, not any one master's copy. + if runningMasters(mc) >= 2 { + id, err := mc.WaitForTopologyId(leaderElectionTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("round %d (seed %d): %v", round, seed, err) + } + if id != topologyId { + mc.DumpLogs() + t.Fatalf("round %d (seed %d): TopologyId changed from %s to %s", round, seed, topologyId, id) + } + } + } + + // Everything back up, so the walk ends on a full cluster. + for i := range 3 { + mc.StartNode(i) + } + if _, err := waitForCommonLeader(mc, leaderElectionTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("cluster did not recover after the walk (seed %d): %v", seed, err) + } + for i := range 3 { + if err := waitForPeerCount(mc, i, 2, leaderElectionTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("master %d does not see the full cluster (seed %d): %v", i, seed, err) + } + } + id, err := mc.WaitForTopologyId(leaderElectionTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("no TopologyId after the walk (seed %d): %v", seed, err) + } + if id != topologyId { + mc.DumpLogs() + t.Fatalf("TopologyId changed from %s to %s over the walk (seed %d)", topologyId, id, seed) + } + }) + } +} + +// waitForSettledElection waits for a quorum to agree on one leader, and fails +// if two masters claim leadership across consecutive polls — anything shorter +// than that is a master on its way down. +// +// Below a quorum there is nothing to wait for: the walk moves on. A master that +// has lost its quorum can keep claiming leadership for tens of seconds under +// goraft, and it cannot commit anything in that window, so stepping down is a +// liveness question rather than a safety one. TestTwoMastersDownAndRestart +// holds that direction to account. +func waitForSettledElection(mc *MasterCluster, timeout time.Duration) error { + if runningMasters(mc) < 2 { + return nil + } + + var lastErr error + splitPolls := 0 + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + claims, err := leaderClaims(mc) + if err != nil { + lastErr = err + } + if len(claims) > 1 { + splitPolls++ + if splitPolls > 1 { + return fmt.Errorf("masters %v all claim leadership", claims) + } + time.Sleep(waitTick) + continue + } + splitPolls = 0 + + if _, err := commonLeader(mc); err == nil { + return nil + } else { + lastErr = err + } + time.Sleep(waitTick) + } + if lastErr == nil { + lastErr = fmt.Errorf("cluster did not settle within %v", timeout) + } + return lastErr +} + +// leaderClaims returns the running masters that call themselves leader. The +// error reports masters that did not answer at all, which is a reason to keep +// waiting rather than a verdict. +func leaderClaims(mc *MasterCluster) (claims []int, err error) { + for i := range 3 { + if !mc.IsNodeRunning(i) { + continue + } + cs, statusErr := mc.GetClusterStatus(i) + if statusErr != nil { + err = fmt.Errorf("master %d: %w", i, statusErr) + continue + } + if cs.IsLeader { + claims = append(claims, i) + } + } + return claims, err +} + +func runningMasters(mc *MasterCluster) int { + count := 0 + for i := range 3 { + if mc.IsNodeRunning(i) { + count++ + } + } + return count +} + +// chaosSeed picks the walk. It is random by default and always logged, so a +// failure names the seed that reproduces it. +func chaosSeed(t *testing.T) uint64 { + t.Helper() + if v := os.Getenv("MULTI_MASTER_IT_SEED"); v != "" { + seed, err := strconv.ParseUint(v, 10, 64) + if err != nil { + t.Fatalf("MULTI_MASTER_IT_SEED %q: %v", v, err) + } + return seed + } + return uint64(time.Now().UnixNano()) +} diff --git a/test/multi_master/cluster.go b/test/multi_master/cluster.go index f216f5d16..aeeae9996 100644 --- a/test/multi_master/cluster.go +++ b/test/multi_master/cluster.go @@ -33,6 +33,9 @@ type masterNode struct { cmd *exec.Cmd logFile string stopped bool + // peersStr overrides the cluster-wide peer list for this node, so a test + // can start a master that only knows about a subset of the cluster. + peersStr string } // MasterCluster manages a 3-node master raft cluster for integration tests. @@ -48,6 +51,9 @@ type MasterCluster struct { // peers string shared by all nodes, e.g. "127.0.0.1:9333,127.0.0.1:9334,127.0.0.1:9335" peersStr string + + // raftHashicorp starts the masters with -raftHashicorp + raftHashicorp bool } // clusterStatus is the JSON returned by /cluster/status. @@ -61,6 +67,35 @@ type clusterStatus struct { func StartMasterCluster(t testing.TB) *MasterCluster { t.Helper() + mc := NewMasterCluster(t, false) + for i := range 3 { + mc.StartNode(i) + } + + if err := mc.WaitForLeader(waitTimeout); err != nil { + mc.DumpLogs() + mc.StopAll() + t.Fatalf("cluster did not elect a leader: %v", err) + } + + // Wait for TopologyId to be generated and propagated. This is async + // after leader election, and we need it committed before tests can + // reliably stop/restart nodes. + if _, err := mc.WaitForTopologyId(waitTimeout); err != nil { + mc.DumpLogs() + mc.StopAll() + t.Fatalf("TopologyId not generated: %v", err) + } + + return mc +} + +// NewMasterCluster allocates ports and data directories for a 3-node master +// cluster without starting anything, so a test can choose what each node comes +// up with. +func NewMasterCluster(t testing.TB, raftHashicorp bool) *MasterCluster { + t.Helper() + weedBinary, err := findOrBuildWeedBinary() if err != nil { t.Fatalf("resolve weed binary: %v", err) @@ -94,32 +129,14 @@ func StartMasterCluster(t testing.TB) *MasterCluster { } mc := &MasterCluster{ - t: t, - weedBinary: weedBinary, - baseDir: baseDir, - logsDir: logsDir, - keepLogs: keepLogs, - nodes: nodes, - peersStr: strings.Join(peerParts, ","), - } - - for i := range 3 { - mc.StartNode(i) - } - - if err := mc.WaitForLeader(waitTimeout); err != nil { - mc.DumpLogs() - mc.StopAll() - t.Fatalf("cluster did not elect a leader: %v", err) - } - - // Wait for TopologyId to be generated and propagated. This is async - // after leader election, and we need it committed before tests can - // reliably stop/restart nodes. - if err := mc.WaitForTopologyId(waitTimeout); err != nil { - mc.DumpLogs() - mc.StopAll() - t.Fatalf("TopologyId not generated: %v", err) + t: t, + weedBinary: weedBinary, + baseDir: baseDir, + logsDir: logsDir, + keepLogs: keepLogs, + nodes: nodes, + peersStr: strings.Join(peerParts, ","), + raftHashicorp: raftHashicorp, } t.Cleanup(func() { @@ -128,6 +145,14 @@ func StartMasterCluster(t testing.TB) *MasterCluster { return mc } +// SetNodePeers narrows the peer list node i starts with, mirroring a +// StatefulSet whose replica count changed under a running master. +func (mc *MasterCluster) SetNodePeers(i int, peers string) { + mc.mu.Lock() + defer mc.mu.Unlock() + mc.nodes[i].peersStr = peers +} + // StartNode starts the master process at the given index (0–2). func (mc *MasterCluster) StartNode(i int) { mc.t.Helper() @@ -144,17 +169,24 @@ func (mc *MasterCluster) StartNode(i int) { mc.t.Fatalf("create log for node %d: %v", i, err) } + peersStr := n.peersStr + if peersStr == "" { + peersStr = mc.peersStr + } args := []string{ "master", "-ip=127.0.0.1", "-port=" + strconv.Itoa(n.port), "-port.grpc=" + strconv.Itoa(n.grpcPort), "-mdir=" + n.dataDir, - "-peers=" + mc.peersStr, + "-peers=" + peersStr, "-electionTimeout=3s", "-volumeSizeLimitMB=32", "-defaultReplication=000", } + if mc.raftHashicorp { + args = append(args, "-raftHashicorp") + } n.cmd = exec.Command(mc.weedBinary, args...) n.cmd.Dir = mc.baseDir @@ -292,6 +324,24 @@ func (mc *MasterCluster) WaitForLeader(timeout time.Duration) error { return fmt.Errorf("no leader elected within %v", timeout) } +// WaitForNoLeader waits until no running master claims leadership. goraft only +// checks whether it still has a quorum on an election-timeout ticker, and needs +// its peers to go quiet for a full timeout first, so a master that has lost its +// quorum can keep claiming leadership for tens of seconds. It cannot commit +// anything in that window. +func (mc *MasterCluster) WaitForNoLeader(timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + idx, _ := mc.FindLeader() + if idx < 0 { + return nil + } + time.Sleep(waitTick) + } + idx, addr := mc.FindLeader() + return fmt.Errorf("master %d at %s still claims leadership after %v", idx, addr, timeout) +} + // WaitForNewLeader waits for a leader that is different from the given address. func (mc *MasterCluster) WaitForNewLeader(oldLeaderAddr string, timeout time.Duration) (int, string, error) { deadline := time.Now().Add(timeout) @@ -305,18 +355,20 @@ func (mc *MasterCluster) WaitForNewLeader(oldLeaderAddr string, timeout time.Dur return -1, "", fmt.Errorf("no new leader (different from %s) within %v", oldLeaderAddr, timeout) } -// WaitForTopologyId waits until the leader reports a non-empty TopologyId. -func (mc *MasterCluster) WaitForTopologyId(timeout time.Duration) error { +// WaitForTopologyId waits until the leader reports a non-empty TopologyId, and +// returns it. It is only readable once the leader has applied the raft entry +// carrying it, which lands after the election it won. +func (mc *MasterCluster) WaitForTopologyId(timeout time.Duration) (string, error) { deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { if idx, _ := mc.FindLeader(); idx >= 0 { if id, err := mc.GetTopologyId(idx); err == nil && id != "" { - return nil + return id, nil } } time.Sleep(waitTick) } - return fmt.Errorf("TopologyId not available within %v", timeout) + return "", fmt.Errorf("TopologyId not available within %v", timeout) } // WaitForNodeReady waits for node i to respond to HTTP. diff --git a/test/multi_master/failover_test.go b/test/multi_master/failover_test.go index 45ba204d4..9b491c89c 100644 --- a/test/multi_master/failover_test.go +++ b/test/multi_master/failover_test.go @@ -9,7 +9,12 @@ import ( const ( // Election timeout is 3s in our cluster config; allow generous margin. - leaderElectionTimeout = 20 * time.Second + leaderElectionTimeout = 30 * time.Second + + // Losing a quorum is much slower to show than winning one. goraft only + // notices on an election-timeout ticker, after its peers have been quiet + // for a full timeout, and has been measured taking over 30s to step down. + leaderStepDownTimeout = 60 * time.Second ) // TestLeaderDownAndRecoverQuickly verifies that when the leader is stopped and @@ -149,20 +154,11 @@ func TestTwoMastersDownAndRestart(t *testing.T) { mc.StopNode(down1) mc.StopNode(down2) - // The surviving node alone cannot form a quorum — no leader expected. - // Wait long enough for any stale leadership to expire (election timeout - // is 3s in our config, quorum check fires every election timeout). - time.Sleep(5 * time.Second) - soloLeaderIdx, _ := mc.FindLeader() - if soloLeaderIdx >= 0 { - // It's possible the survivor briefly thinks it's leader before stepping down. - // Give it time to realize it lost quorum. - time.Sleep(5 * time.Second) - soloLeaderIdx, _ = mc.FindLeader() - } - if soloLeaderIdx >= 0 { + // The surviving node alone cannot form a quorum, so it has to give up the + // leadership it is still claiming. + if err := mc.WaitForNoLeader(leaderStepDownTimeout); err != nil { mc.DumpLogs() - t.Fatalf("expected no leader with only 1 of 3 nodes, but node %d claims leadership", soloLeaderIdx) + t.Fatalf("expected no leader with only 1 of 3 nodes: %v", err) } // Restart both downed nodes. diff --git a/test/multi_master/join_test.go b/test/multi_master/join_test.go new file mode 100644 index 000000000..d56eeaa8c --- /dev/null +++ b/test/multi_master/join_test.go @@ -0,0 +1,153 @@ +package multi_master + +import ( + "fmt" + "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/pb" +) + +var raftImplementations = []struct { + name string + raftHashicorp bool +}{ + {"goraft", false}, + {"hashicorp", true}, +} + +// TestFreshClusterFormsWithoutALeader covers the other half of the bootstrap +// decision: with no leader anywhere, three masters starting together still have +// to mint a cluster between them. +func TestFreshClusterFormsWithoutALeader(t *testing.T) { + for _, impl := range raftImplementations { + t.Run(impl.name, func(t *testing.T) { + mc := NewMasterCluster(t, impl.raftHashicorp) + for i := range 3 { + mc.StartNode(i) + } + + if _, err := waitForCommonLeader(mc, waitTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("fresh cluster did not converge: %v", err) + } + for i := range 3 { + if err := waitForPeerCount(mc, i, 2, waitTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("master %d does not see the full cluster: %v", i, err) + } + } + }) + } +} + +// TestScaleUpOntoExistingLeader mirrors a Kubernetes master StatefulSet whose +// replica count goes back from one to three: master 0 keeps running as the +// leader of a single-peer cluster while two fresh masters come up pointing at +// all three. The newcomers start with an empty raft log, so neither raft +// implementation lets them campaign — the sitting leader has to admit them. +func TestScaleUpOntoExistingLeader(t *testing.T) { + for _, impl := range raftImplementations { + t.Run(impl.name, func(t *testing.T) { + mc := NewMasterCluster(t, impl.raftHashicorp) + + // Master 0 is alone in its peer list, the way the operator renders + // -peers when spec.master.replicas is 1. + mc.SetNodePeers(0, mc.NodeAddress(0)) + mc.StartNode(0) + if err := mc.WaitForLeader(waitTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("single master did not become leader: %v", err) + } + + // Scale up. These two carry the full peer list; master 0 still + // runs with the old one and has never heard of them. + mc.StartNode(1) + mc.StartNode(2) + + leader, err := waitForCommonLeader(mc, waitTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("masters did not converge after scaling up: %v", err) + } + if leader != mc.NodeAddress(0) { + t.Fatalf("leader moved to %s, want the sitting leader %s", leader, mc.NodeAddress(0)) + } + + if err := waitForPeerCount(mc, 0, 2, waitTimeout); err != nil { + mc.DumpLogs() + t.Fatalf("leader did not admit both new masters: %v", err) + } + }) + } +} + +// waitForCommonLeader waits until every running master names the same leader, +// and returns it. +func waitForCommonLeader(mc *MasterCluster, timeout time.Duration) (string, error) { + var lastErr error + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + leader, err := commonLeader(mc) + if err == nil { + return leader, nil + } + lastErr = err + time.Sleep(waitTick) + } + return "", lastErr +} + +func commonLeader(mc *MasterCluster) (string, error) { + agreed := "" + for i := range 3 { + if !mc.IsNodeRunning(i) { + continue + } + cs, err := mc.GetClusterStatus(i) + if err != nil { + return "", err + } + leader := pb.ServerAddress(cs.Leader).ToHttpAddress() + if leader == "" { + return "", fmt.Errorf("master %d has no leader", i) + } + if agreed == "" { + agreed = leader + } else if agreed != leader { + return "", fmt.Errorf("masters disagree on the leader: %s and %s", agreed, leader) + } + } + if agreed == "" { + return "", fmt.Errorf("no master is running") + } + return agreed, nil +} + +// waitForPeerCount waits until node i reports the given number of raft peers. +// The count excludes the node itself. +func waitForPeerCount(mc *MasterCluster, i, want int, timeout time.Duration) error { + got := -1 + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + cs, err := mc.GetClusterStatus(i) + if err == nil { + got = peerCountExcludingSelf(cs.Peers, mc.NodeAddress(i)) + if got == want { + return nil + } + } + time.Sleep(waitTick) + } + return fmt.Errorf("master %d reports %d peers, want %d", i, got, want) +} + +func peerCountExcludingSelf(peers []string, self string) int { + count := 0 + for _, peer := range peers { + if pb.ServerAddress(peer).ToHttpAddress() != self { + count++ + } + } + return count +} diff --git a/weed/command/master.go b/weed/command/master.go index 454f4aada..ebe5b8f0d 100644 --- a/weed/command/master.go +++ b/weed/command/master.go @@ -234,15 +234,17 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { if raftServer == nil { glog.Fatalf("please verify %s is writable, see https://github.com/seaweedfs/seaweedfs/issues/717: %s", *masterOption.metaFolder, err) } - // For single-master mode with a fresh log, initialize cluster immediately. - // When resuming with existing state, the server is already a member and - // will self-elect via fastResume — sending another JoinCommand would block - // because goraft's setCommitIndex returns early on JoinCommand entries, - // preventing the new entry's event from being notified when old uncommitted - // JoinCommands exist in the log. - if isSingleMaster && !raftServer.HasExistingState() { - glog.V(0).Infof("Single-master mode: initializing cluster immediately") - raftServer.DoJoinCommand() + } + // For single-master mode with a fresh log, initialize cluster immediately. + // When resuming with existing state, the server is already a member and + // will self-elect via fastResume — sending another JoinCommand would block + // because goraft's setCommitIndex returns early on JoinCommand entries, + // preventing the new entry's event from being notified when old uncommitted + // JoinCommands exist in the log. + if isSingleMaster && !raftServer.HasExistingState() { + glog.V(0).Infof("Single-master mode: initializing cluster immediately") + if err := raftServer.Bootstrap(); err != nil { + glog.Errorf("fail to bootstrap the cluster: %v", err) } } ms.SetRaftServer(raftServer) @@ -272,8 +274,13 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { go grpcS.Serve(grpcL) pb.ServeGrpcOnLocalSocket(grpcS, grpcPort) - // For multi-master mode with non-Hashicorp raft, wait and check if we should join - if !*masterOption.raftHashicorp && !isSingleMaster { + // A master that starts with no raft state cannot elect on its own — neither + // raft implementation lets a server outside the configuration campaign — so + // it has to be pulled in by a leader. Keep asking the peers who the leader is + // until we are in: the leader admits us once our master client registers, and + // only when nobody has one does the first peer mint a new cluster. Restarting + // a master alone, or scaling the peer list up, both land here. + if !isSingleMaster { go func() { // Stagger bootstrap by peer index so masters don't all check // simultaneously. Peer 0 waits ~1.5s, peer 1 ~3s, etc. @@ -282,22 +289,24 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { glog.V(0).Infof("bootstrap check in %v (peer index %d of %d)", delay, idx, len(peers)) time.Sleep(delay) - ms.Topo.RaftServerAccessLock.RLock() - isEmptyMaster := ms.Topo.RaftServer.Leader() == "" && ms.Topo.RaftServer.IsLogEmpty() - isFirst := idx == 0 - if isEmptyMaster && isFirst { - existingLeader := ms.MasterClient.FindLeaderFromOtherPeers(myMasterAddress) - if existingLeader == "" { - raftServer.DoJoinCommand() - } else { - glog.V(0).Infof("skip bootstrap: existing leader %s found from peers", existingLeader) + for { + if raftServer.HasExistingState() { + return } - } else if !isEmptyMaster { - glog.V(0).Infof("skip bootstrap: leader=%q logEmpty=%v", ms.Topo.RaftServer.Leader(), ms.Topo.RaftServer.IsLogEmpty()) - } else { - glog.V(0).Infof("skip bootstrap: %v is not the first master in peers (index %d)", myMasterAddress, idx) + if leader, err := ms.Topo.MaybeLeader(); err == nil && leader != "" { + return + } + if existingLeader := ms.MasterClient.FindLeaderFromOtherPeers(myMasterAddress); existingLeader != "" { + glog.V(0).Infof("waiting to be admitted by existing leader %s", existingLeader) + } else if idx == 0 { + if err := raftServer.Bootstrap(); err != nil { + glog.Errorf("fail to bootstrap the cluster: %v", err) + } + } else { + glog.V(0).Infof("skip bootstrap: %v is not the first master in peers (index %d)", myMasterAddress, idx) + } + time.Sleep(raftJoinCheckDelay) } - ms.Topo.RaftServerAccessLock.RUnlock() }() } diff --git a/weed/server/master_grpc_server.go b/weed/server/master_grpc_server.go index 2f2a89b81..d45ee860e 100644 --- a/weed/server/master_grpc_server.go +++ b/weed/server/master_grpc_server.go @@ -139,10 +139,12 @@ func (ms *MasterServer) SendHeartbeat(stream master_pb.Seaweed_SendHeartbeatServ } if !ms.Topo.IsLeader() { - // tell the volume servers about the leader - newLeader, err := ms.Topo.Leader() - if err != nil { - glog.Warningf("SendHeartbeat find leader: %v", err) + // tell the volume servers about the leader we know of right now, so + // that a follower without one hands the heartbeat back immediately + // instead of holding it through an election + newLeader, err := ms.Topo.MaybeLeader() + if err != nil || newLeader == "" { + glog.V(1).Infof("SendHeartbeat find leader: %v", err) return raft.NotLeaderError } if err := stream.Send(&master_pb.HeartbeatResponse{ @@ -401,6 +403,13 @@ func (ms *MasterServer) KeepConnected(stream master_pb.Seaweed_KeepConnectedServ if req.ClientType == cluster.FilerType { ms.LockRingManager.AddServer(cluster.FilerGroupName(req.FilerGroup), peerAddress) } + if req.ClientType == cluster.MasterType { + // Only the leader gets this far, and a master that starts with no raft + // state cannot campaign its way in, so this registration is where it + // joins the quorum. The broadcast below is not enough: it only reaches + // masters already connected to us. + ms.AdmitRaftPeer(peerAddress) + } defer func() { for _, update := range ms.Cluster.RemoveClusterNode(req.FilerGroup, req.ClientType, peerAddress) { @@ -539,9 +548,12 @@ func (ms *MasterServer) broadcastVolumeLocationsToClients(locations []*master_pb } func (ms *MasterServer) informNewLeader(stream master_pb.Seaweed_KeepConnectedServer) error { - leader, err := ms.Topo.Leader() - if err != nil { - glog.Errorf("topo leader: %v", err) + // Answer from what raft knows now. Waiting out an election here pins the + // client to a master that cannot serve it, right when it should be moving + // on to the next peer to find the one that can. + leader, err := ms.Topo.MaybeLeader() + if err != nil || leader == "" { + glog.V(1).Infof("topo leader: %v", err) return raft.NotLeaderError } if err := stream.Send(&master_pb.KeepConnectedResponse{ @@ -606,7 +618,7 @@ func findClientAddress(ctx context.Context, grpcPort uint32) string { func (ms *MasterServer) GetMasterConfiguration(ctx context.Context, req *master_pb.GetMasterConfigurationRequest) (*master_pb.GetMasterConfigurationResponse, error) { // tell the volume servers about the leader - leader, _ := ms.Topo.Leader() + leader, _ := ms.Topo.MaybeLeader() // MIGRATION: expose maintenance scripts for admin server seeding. Remove after March 2027. v := util.GetViper() diff --git a/weed/server/master_grpc_server_raft.go b/weed/server/master_grpc_server_raft.go index f4bf85988..1dcfac4e0 100644 --- a/weed/server/master_grpc_server_raft.go +++ b/weed/server/master_grpc_server_raft.go @@ -6,6 +6,7 @@ import ( "net" "github.com/hashicorp/raft" + goraft "github.com/seaweedfs/raft" "google.golang.org/grpc/codes" "google.golang.org/grpc/peer" "google.golang.org/grpc/status" @@ -96,6 +97,62 @@ func (ms *MasterServer) RaftListClusterServers(ctx context.Context, req *master_ return resp, nil } +// raftAddServer admits a master into the quorum. A master that starts with no +// raft state cannot elect on its own, so the leader has to pull it in; this is +// the one place that knows how to do that for either raft implementation. +// goraft has no non-voting members, so an admitted peer always votes there. +func (ms *MasterServer) raftAddServer(id string, grpcAddress string, voter bool) error { + ms.Topo.RaftServerAccessLock.RLock() + defer ms.Topo.RaftServerAccessLock.RUnlock() + + if ms.Topo.HashicorpRaft != nil { + if ms.Topo.HashicorpRaft.State() != raft.Leader { + return fmt.Errorf("raft add server %s failed: %s is no current leader", id, ms.Topo.HashicorpRaft.String()) + } + var idxFuture raft.IndexFuture + if voter { + idxFuture = ms.Topo.HashicorpRaft.AddVoter(raft.ServerID(id), raft.ServerAddress(grpcAddress), 0, 0) + } else { + idxFuture = ms.Topo.HashicorpRaft.AddNonvoter(raft.ServerID(id), raft.ServerAddress(grpcAddress), 0, 0) + } + return idxFuture.Error() + } + + if ms.Topo.RaftServer == nil { + return nil + } + if ms.Topo.RaftServer.State() != goraft.Leader { + return fmt.Errorf("raft add server %s failed: %s is no current leader", id, ms.Topo.RaftServer.Name()) + } + _, err := ms.Topo.RaftServer.Do(&goraft.DefaultJoinCommand{ + Name: id, + ConnectionString: grpcAddress, + }) + return err +} + +// raftRemoveServer drops a master from the quorum. +func (ms *MasterServer) raftRemoveServer(id string) error { + ms.Topo.RaftServerAccessLock.RLock() + defer ms.Topo.RaftServerAccessLock.RUnlock() + + if ms.Topo.HashicorpRaft != nil { + if ms.Topo.HashicorpRaft.State() != raft.Leader { + return fmt.Errorf("raft remove server %s failed: %s is no current leader", id, ms.Topo.HashicorpRaft.String()) + } + return ms.Topo.HashicorpRaft.RemoveServer(raft.ServerID(id), 0, 0).Error() + } + + if ms.Topo.RaftServer == nil { + return nil + } + if ms.Topo.RaftServer.State() != goraft.Leader { + return fmt.Errorf("raft remove server %s failed: %s is no current leader", id, ms.Topo.RaftServer.Name()) + } + _, err := ms.Topo.RaftServer.Do(&goraft.DefaultLeaveCommand{Name: id}) + return err +} + func (ms *MasterServer) RaftAddServer(ctx context.Context, req *master_pb.RaftAddServerRequest) (*master_pb.RaftAddServerResponse, error) { resp := &master_pb.RaftAddServerResponse{} @@ -103,25 +160,7 @@ func (ms *MasterServer) RaftAddServer(ctx context.Context, req *master_pb.RaftAd return resp, err } - ms.Topo.RaftServerAccessLock.RLock() - defer ms.Topo.RaftServerAccessLock.RUnlock() - - if ms.Topo.HashicorpRaft == nil { - return resp, nil - } - - if ms.Topo.HashicorpRaft.State() != raft.Leader { - return nil, fmt.Errorf("raft add server %s failed: %s is no current leader", req.Id, ms.Topo.HashicorpRaft.String()) - } - - var idxFuture raft.IndexFuture - if req.Voter { - idxFuture = ms.Topo.HashicorpRaft.AddVoter(raft.ServerID(req.Id), raft.ServerAddress(req.Address), 0, 0) - } else { - idxFuture = ms.Topo.HashicorpRaft.AddNonvoter(raft.ServerID(req.Id), raft.ServerAddress(req.Address), 0, 0) - } - - if err := idxFuture.Error(); err != nil { + if err := ms.raftAddServer(req.Id, req.Address, req.Voter); err != nil { return nil, err } return resp, nil @@ -134,17 +173,6 @@ func (ms *MasterServer) RaftRemoveServer(ctx context.Context, req *master_pb.Raf return resp, err } - ms.Topo.RaftServerAccessLock.RLock() - defer ms.Topo.RaftServerAccessLock.RUnlock() - - if ms.Topo.HashicorpRaft == nil { - return resp, nil - } - - if ms.Topo.HashicorpRaft.State() != raft.Leader { - return nil, fmt.Errorf("raft remove server %s failed: %s is no current leader", req.Id, ms.Topo.HashicorpRaft.String()) - } - if !req.Force { ms.clientChansLock.RLock() _, ok := ms.clientChans[fmt.Sprintf("%s@%s", cluster.MasterType, req.Id)] @@ -154,8 +182,7 @@ func (ms *MasterServer) RaftRemoveServer(ctx context.Context, req *master_pb.Raf } } - idxFuture := ms.Topo.HashicorpRaft.RemoveServer(raft.ServerID(req.Id), 0, 0) - if err := idxFuture.Error(); err != nil { + if err := ms.raftRemoveServer(req.Id); err != nil { return nil, err } return resp, nil diff --git a/weed/server/master_server.go b/weed/server/master_server.go index 4677c69da..401e2ec37 100644 --- a/weed/server/master_server.go +++ b/weed/server/master_server.go @@ -82,6 +82,9 @@ type MasterServer struct { topologyIdGenLock sync.Mutex + // masters currently being admitted into the raft quorum, keyed by raft id + raftPeerAdmissions sync.Map + MasterClient *wdclient.MasterClient adminLocks *AdminLocks @@ -216,9 +219,10 @@ func (ms *MasterServer) healthzHandler(w http.ResponseWriter, r *http.Request) { } func (ms *MasterServer) readyzHandler(w http.ResponseWriter, r *http.Request) { - // Readiness: check we can serve traffic. - leader, err := ms.Topo.Leader() - if err != nil { + // Readiness: check we can serve traffic. Answer from what raft knows now + // rather than waiting out an election, so the probe's own timeout decides. + leader, err := ms.Topo.MaybeLeader() + if err != nil || leader == "" { w.WriteHeader(http.StatusServiceUnavailable) return } @@ -499,51 +503,115 @@ func (ms *MasterServer) createSequencer(option *MasterOption) sequence.Sequencer } func (ms *MasterServer) OnPeerUpdate(update *master_pb.ClusterNodeUpdate, startFrom time.Time) { - ms.Topo.RaftServerAccessLock.RLock() - defer ms.Topo.RaftServerAccessLock.RUnlock() - - if update.NodeType != cluster.MasterType || ms.Topo.HashicorpRaft == nil { + if update.NodeType != cluster.MasterType { return } glog.V(4).Infof("OnPeerUpdate: %+v", update) peerAddress := pb.ServerAddress(update.Address) - peerName := raftServerID(peerAddress) - if ms.Topo.HashicorpRaft.State() != hashicorpRaft.Leader { + if update.IsAdd { + ms.AdmitRaftPeer(peerAddress) return } - if update.IsAdd { - raftServerFound := false - for _, server := range ms.Topo.HashicorpRaft.GetConfiguration().Configuration().Servers { - if string(server.ID) == peerName { - raftServerFound = true - } - } - if !raftServerFound { - glog.V(0).Infof("adding new raft server: %s", peerName) - ms.Topo.HashicorpRaft.AddVoter( - hashicorpRaft.ServerID(peerName), - hashicorpRaft.ServerAddress(peerAddress.ToGrpcAddress()), 0, 0) - } - } else { - pb.WithMasterClient(context.Background(), false, peerAddress, ms.grpcDialOption, true, func(client master_pb.SeaweedClient) error { - ctx, cancel := context.WithTimeout(context.TODO(), 15*time.Second) - defer cancel() - if _, err := client.Ping(ctx, &master_pb.PingRequest{Target: string(peerAddress), TargetType: cluster.MasterType}); err != nil { - glog.V(0).Infof("master %s didn't respond to pings. remove raft server", peerName) - // We are the leader here, so drop the dead peer through the local - // raft handle, mirroring the AddVoter branch above, instead of - // dialing our own RaftRemoveServer RPC. - if err := ms.Topo.HashicorpRaft.RemoveServer(hashicorpRaft.ServerID(peerName), 0, 0).Error(); err != nil { - glog.Warningf("failed removing old raft server: %v", err) - return err - } - } else { - glog.V(0).Infof("master %s successfully responded to ping", peerName) - } - return nil - }) + + ms.Topo.RaftServerAccessLock.RLock() + defer ms.Topo.RaftServerAccessLock.RUnlock() + + // goraft rebuilds its peer set from -peers on every start, so a departed + // master is already out of the list there and would come back on the next + // restart anyway; only hashicorp raft carries membership across restarts. + if ms.Topo.HashicorpRaft == nil || ms.Topo.HashicorpRaft.State() != hashicorpRaft.Leader { + return } + // A master that is merely down is still a member: -peers is what declares + // membership, and updatePeers reconciles the configuration against it on + // every leadership change. Evicting one here would shrink the quorum behind + // the operator's back, and a restart then races the eviction — the master + // gets re-admitted, the removal lands after it, and it is left out of the + // configuration with nobody left to vote it back in. + for _, peer := range ms.MasterClient.GetMasters(context.Background()) { + if peer.ToHttpAddress() == peerAddress.ToHttpAddress() { + return + } + } + + peerName := raftServerID(peerAddress) + pb.WithMasterClient(context.Background(), false, peerAddress, ms.grpcDialOption, true, func(client master_pb.SeaweedClient) error { + ctx, cancel := context.WithTimeout(context.TODO(), 15*time.Second) + defer cancel() + if _, err := client.Ping(ctx, &master_pb.PingRequest{Target: string(peerAddress), TargetType: cluster.MasterType}); err != nil { + glog.V(0).Infof("master %s didn't respond to pings. remove raft server", peerName) + // We are the leader here, so drop the dead peer through the local + // raft handle instead of dialing our own RaftRemoveServer RPC. + if err := ms.Topo.HashicorpRaft.RemoveServer(hashicorpRaft.ServerID(peerName), 0, 0).Error(); err != nil { + glog.Warningf("failed removing old raft server: %v", err) + return err + } + } else { + glog.V(0).Infof("master %s successfully responded to ping", peerName) + } + return nil + }) +} + +// AdmitRaftPeer pulls a master into the raft quorum, if we are the leader and +// do not have it yet. A master that starts with no raft state cannot campaign +// under either implementation, so this is its only way in. +func (ms *MasterServer) AdmitRaftPeer(peerAddress pb.ServerAddress) { + if peerAddress.ToHttpAddress() == ms.option.Master.ToHttpAddress() { + return + } + + peerName, ok := ms.missingRaftPeerName(peerAddress) + if !ok { + return + } + if _, alreadyAdmitting := ms.raftPeerAdmissions.LoadOrStore(peerName, struct{}{}); alreadyAdmitting { + return + } + glog.V(0).Infof("adding new raft server: %s", peerName) + // The join commits through raft, which waits on the other peers, so keep it + // off the caller: this runs on the peer update stream and on the grpc + // handler that a joining master is still blocked in. + go func() { + defer ms.raftPeerAdmissions.Delete(peerName) + if err := ms.raftAddServer(peerName, peerAddress.ToGrpcAddress(), true); err != nil { + glog.Warningf("failed adding raft server %s: %v", peerName, err) + } + }() +} + +// missingRaftPeerName returns the raft id to admit peerAddress under, and +// whether this master is the leader and is missing that peer. +func (ms *MasterServer) missingRaftPeerName(peerAddress pb.ServerAddress) (string, bool) { + ms.Topo.RaftServerAccessLock.RLock() + defer ms.Topo.RaftServerAccessLock.RUnlock() + + if ms.Topo.RaftServer != nil { + if ms.Topo.RaftServer.State() != raft.Leader { + return "", false + } + // Peers are keyed by the name a master calls itself, which carries the + // grpc port; match on the http address so a peer already known under + // another spelling is not added twice. + for name := range ms.Topo.RaftServer.Peers() { + if pb.ServerAddress(name).ToHttpAddress() == peerAddress.ToHttpAddress() { + return "", false + } + } + return string(peerAddress), true + } + + if ms.Topo.HashicorpRaft == nil || ms.Topo.HashicorpRaft.State() != hashicorpRaft.Leader { + return "", false + } + peerName := raftServerID(peerAddress) + for _, server := range ms.Topo.HashicorpRaft.GetConfiguration().Configuration().Servers { + if string(server.ID) == peerName { + return "", false + } + } + return peerName, true } func (ms *MasterServer) Shutdown() { diff --git a/weed/server/raft_hashicorp.go b/weed/server/raft_hashicorp.go index 575478aec..7734fb364 100644 --- a/weed/server/raft_hashicorp.go +++ b/weed/server/raft_hashicorp.go @@ -11,7 +11,6 @@ import ( "os" "path" "path/filepath" - "sort" "time" transport "github.com/Jille/raft-grpc-transport" @@ -33,26 +32,6 @@ const ( updatePeersTimeout = 15 * time.Minute ) -func getPeerIdx(self pb.ServerAddress, mapPeers map[string]pb.ServerAddress) int { - peerIDs := make([]string, 0, len(mapPeers)) - seen := make(map[string]struct{}, len(mapPeers)) - for _, peer := range mapPeers { - id := raftServerID(peer) - if _, ok := seen[id]; ok { - continue - } - seen[id] = struct{}{} - peerIDs = append(peerIDs, id) - } - sort.Strings(peerIDs) - selfID := raftServerID(self) - idx := sort.SearchStrings(peerIDs, selfID) - if idx < len(peerIDs) && peerIDs[idx] == selfID { - return idx - } - return -1 -} - func raftServerID(server pb.ServerAddress) string { return server.ToHttpAddress() } @@ -225,20 +204,15 @@ func NewHashicorpRaftServer(option *RaftServerOption) (*RaftServer, error) { return nil, fmt.Errorf("raft.NewRaft: %w", err) } - updatePeers := false - if option.RaftBootstrap || len(s.RaftHashicorp.GetConfiguration().Configuration().Servers) == 0 { - cfg := s.AddPeersConfiguration() - // Need to get lock, in case all servers do this at the same time. - peerIdx := getPeerIdx(s.serverAddr, s.peers) - timeSleep := time.Duration(float64(c.LeaderLeaseTimeout) * (rand.Float64()*0.25 + 1) * float64(peerIdx)) - glog.V(0).Infof("Bootstrapping idx: %d sleep: %v new cluster: %+v", peerIdx, timeSleep, cfg) - time.Sleep(timeSleep) - f := s.RaftHashicorp.BootstrapCluster(cfg) - if err := f.Error(); err != nil { + // An explicit -raftBootstrap mints the cluster right here. Otherwise the + // caller bootstraps, once it has confirmed no peer already has a leader: + // bootstrapping next to a live leader forms a second cluster instead of + // joining the first one. + updatePeers := len(s.RaftHashicorp.GetConfiguration().Configuration().Servers) > 0 + if option.RaftBootstrap { + if err := s.Bootstrap(); err != nil { return nil, fmt.Errorf("raft.Raft.BootstrapCluster: %w", err) } - } else { - updatePeers = true } go s.monitorLeaderLoop(updatePeers) diff --git a/weed/server/raft_hashicorp_test.go b/weed/server/raft_hashicorp_test.go index e9261e3ac..a33c93f3e 100644 --- a/weed/server/raft_hashicorp_test.go +++ b/weed/server/raft_hashicorp_test.go @@ -35,19 +35,6 @@ func TestRaftServerID(t *testing.T) { } } -func TestGetPeerIdxUsesCanonicalID(t *testing.T) { - peers := map[string]pb.ServerAddress{ - "master-0:9333": pb.ServerAddress("master-0:9333"), - "master-1:9333": pb.ServerAddress("master-1:9333"), - "master-2:9333": pb.ServerAddress("master-2:9333"), - } - self := pb.NewServerAddress("master-2", 9333, 19333) - - if got := getPeerIdx(self, peers); got != 2 { - t.Fatalf("getPeerIdx(%q) = %d, want 2", self, got) - } -} - func TestAddPeersConfigurationUsesCanonicalIDs(t *testing.T) { rs := &RaftServer{ peers: map[string]pb.ServerAddress{ diff --git a/weed/server/raft_server.go b/weed/server/raft_server.go index c98d43250..ef6799512 100644 --- a/weed/server/raft_server.go +++ b/weed/server/raft_server.go @@ -303,15 +303,22 @@ func (s *RaftServer) HasExistingState() bool { return false } -func (s *RaftServer) DoJoinCommand() { - +// Bootstrap mints a new raft cluster out of the configured peers. Only call it +// when no leader exists anywhere: a master that starts with no raft state and +// finds a leader must be admitted by that leader instead, or the two clusters +// never merge. +func (s *RaftServer) Bootstrap() error { glog.V(0).Infoln("Initializing new cluster") - if _, err := s.raftServer.Do(&raft.DefaultJoinCommand{ + if s.RaftHashicorp != nil { + return s.RaftHashicorp.BootstrapCluster(s.AddPeersConfiguration()).Error() + } + if s.raftServer == nil { + return nil + } + _, err := s.raftServer.Do(&raft.DefaultJoinCommand{ Name: s.raftServer.Name(), ConnectionString: s.serverAddr.ToGrpcAddress(), - }); err != nil { - glog.Errorf("fail to send join command: %v", err) - } - + }) + return err } diff --git a/weed/server/raft_server_handlers.go b/weed/server/raft_server_handlers.go index 0ce7e632b..40eae45ca 100644 --- a/weed/server/raft_server_handlers.go +++ b/weed/server/raft_server_handlers.go @@ -24,15 +24,18 @@ func (s *RaftServer) StatusHandler(w http.ResponseWriter, r *http.Request) { MaxVolumeId: s.topo.GetMaxVolumeId(), } - if leader, e := s.topo.Leader(); e == nil { + // Report the leader raft knows right now. Waiting for one to be elected + // holds the response past every health probe's timeout, which is exactly + // when a master is most likely to still be joining. + if leader, e := s.topo.MaybeLeader(); e == nil { ret.Leader = leader } writeJsonQuiet(w, r, http.StatusOK, ret) } func (s *RaftServer) HealthzHandler(w http.ResponseWriter, r *http.Request) { - leader, err := s.topo.Leader() - if err != nil { + leader, err := s.topo.MaybeLeader() + if err != nil || leader == "" { w.WriteHeader(http.StatusServiceUnavailable) return } diff --git a/weed/topology/topology.go b/weed/topology/topology.go index 5cf78d21a..ebaaf5fda 100644 --- a/weed/topology/topology.go +++ b/weed/topology/topology.go @@ -310,16 +310,6 @@ func (t *Topology) Leader() (l pb.ServerAddress, err error) { func() (l pb.ServerAddress, err error) { l, err = t.MaybeLeader() if err == nil && l == "" { - // Thread-safe check if we are the leader - t.RaftServerAccessLock.RLock() - if t.RaftServer != nil && t.RaftServer.State() == raft.Leader { - l = pb.ServerAddress(t.RaftServer.Name()) - } - t.RaftServerAccessLock.RUnlock() - - if l != "" { - return l, nil - } err = leaderNotSelected } return l, err @@ -337,6 +327,9 @@ func (t *Topology) MaybeLeader() (l pb.ServerAddress, err error) { if t.RaftServer != nil { l = pb.ServerAddress(t.RaftServer.Leader()) + if l == "" && t.RaftServer.State() == raft.Leader { + l = pb.ServerAddress(t.RaftServer.Name()) + } } else if t.HashicorpRaft != nil { l = pb.ServerAddress(t.HashicorpRaft.Leader()) } else {