From 173adbc291e7fa09de8573c464c21dd88f1a53ec Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 23 Aug 2026 11:10:20 -0700 Subject: [PATCH] master: never re-seed a raft cluster over committed state under -raftBootstrap (#10883) * master: never re-seed a raft cluster over committed state -raftBootstrap deleted logs.dat, stable.dat and snapshots on every start and then bootstrapped a fresh cluster. Since hashicorp raft only snapshots after 8192 log entries, the TopologyId lives in the log, not in a snapshot, so the pre-wipe snapshot recovery found nothing and each restart minted a new cluster identity. A master that came up while it could not reach its peers seeded a rival cluster; when the two logs met, SetTopologyId's split-brain guard fatally stopped every master holding the other id, and the master layer crash-looped with no quorum. Bootstrapping is genesis. Drop the wipe and the inline bootstrap. The first master in -peers already mints a cluster once it has confirmed no peer has a leader, so the flag has nothing left to do and is now ignored; keeping that one master the sole bootstrap authority is what stops a partition from minting two clusters, so the flag must not widen it either. A master with state rejoins its peers, and one whose data dir was reset is admitted by the sitting leader instead of forking again. * test: cover -raftBootstrap restarts in the multi-master suite Three masters start with -raftBootstrap, the way the helm chart renders it on every master on every roll, and the cluster has to hold one TopologyId after they all restart. /dir/status is proxied to the leader, so each master's own view of the identity is read out of its log, which is where a fork shows up. Before the fix the hashicorp case minted a new id on each restart. --- test/multi_master/cluster.go | 65 ++++++++++++++++++++++++++++++++++ test/multi_master/join_test.go | 59 ++++++++++++++++++++++++++++++ weed/command/master.go | 13 ++++--- weed/server/raft_hashicorp.go | 42 ++-------------------- weed/server/raft_server.go | 4 +-- 5 files changed, 138 insertions(+), 45 deletions(-) diff --git a/test/multi_master/cluster.go b/test/multi_master/cluster.go index aeeae9996..77d218281 100644 --- a/test/multi_master/cluster.go +++ b/test/multi_master/cluster.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" "strconv" "strings" @@ -36,6 +37,9 @@ type masterNode struct { // 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 + // raftBootstrap starts the master with -raftBootstrap, the way the helm + // chart renders master.raftBootstrap on every master, every restart. + raftBootstrap bool } // MasterCluster manages a 3-node master raft cluster for integration tests. @@ -153,6 +157,13 @@ func (mc *MasterCluster) SetNodePeers(i int, peers string) { mc.nodes[i].peersStr = peers } +// SetRaftBootstrap makes node i start with -raftBootstrap. +func (mc *MasterCluster) SetRaftBootstrap(i int) { + mc.mu.Lock() + defer mc.mu.Unlock() + mc.nodes[i].raftBootstrap = true +} + // StartNode starts the master process at the given index (0–2). func (mc *MasterCluster) StartNode(i int) { mc.t.Helper() @@ -187,6 +198,9 @@ func (mc *MasterCluster) StartNode(i int) { if mc.raftHashicorp { args = append(args, "-raftHashicorp") } + if n.raftBootstrap { + args = append(args, "-raftBootstrap") + } n.cmd = exec.Command(mc.weedBinary, args...) n.cmd.Dir = mc.baseDir @@ -386,6 +400,57 @@ func (mc *MasterCluster) WaitForNodeReady(i int, timeout time.Duration) error { return fmt.Errorf("node %d not ready within %v", i, timeout) } +// LogContains reports whether node i's log holds the given text. +func (mc *MasterCluster) LogContains(i int, text string) bool { + b, err := os.ReadFile(mc.nodes[i].logFile) + if err != nil { + return false + } + return strings.Contains(string(b), text) +} + +// topologyIdLine matches every line a master logs when it learns a TopologyId, +// whichever raft implementation applied it. +var topologyIdLine = regexp.MustCompile(`TopologyId[^:]*: ([0-9a-f-]{36})`) + +// NodeTopologyIds returns the TopologyIds node i has logged. /dir/status is +// proxied to the leader, so a master's own view of the cluster identity is only +// visible in its log, and that is where a fork shows up. +func (mc *MasterCluster) NodeTopologyIds(i int) []string { + b, err := os.ReadFile(mc.nodes[i].logFile) + if err != nil { + return nil + } + var ids []string + for _, m := range topologyIdLine.FindAllStringSubmatch(string(b), -1) { + ids = append(ids, m[1]) + } + return ids +} + +// WaitForNodeTopologyIds waits until every master has logged a TopologyId and +// returns what each one saw. +func (mc *MasterCluster) WaitForNodeTopologyIds(timeout time.Duration) ([3][]string, error) { + var ids [3][]string + deadline := time.Now().Add(timeout) + for { + missing := -1 + for i := range 3 { + ids[i] = mc.NodeTopologyIds(i) + if len(ids[i]) == 0 { + missing = i + } + } + if missing < 0 { + return ids, nil + } + if !time.Now().Before(deadline) { + return ids, fmt.Errorf("master %d logged no TopologyId within %v", missing, timeout) + } + time.Sleep(waitTick) + } +} + // DumpLogs prints the tail of all master logs. func (mc *MasterCluster) DumpLogs() { for i := range 3 { diff --git a/test/multi_master/join_test.go b/test/multi_master/join_test.go index d56eeaa8c..f8c847e65 100644 --- a/test/multi_master/join_test.go +++ b/test/multi_master/join_test.go @@ -151,3 +151,62 @@ func peerCountExcludingSelf(peers []string, self string) int { } return count } + +// TestRaftBootstrapKeepsExistingCluster covers a master restarting under +// -raftBootstrap, the way the helm chart renders it on every master on every +// roll. Bootstrapping is genesis: seeding a second cluster over committed raft +// state mints a rival TopologyId, and the split-brain guard then Fatals every +// master that still holds the first one. +func TestRaftBootstrapKeepsExistingCluster(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.SetRaftBootstrap(i) + mc.StartNode(i) + } + before, err := mc.WaitForTopologyId(waitTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("cluster did not mint a TopologyId: %v", err) + } + + for i := range 3 { + mc.StopNode(i) + } + for i := range 3 { + mc.StartNode(i) + } + + after, err := mc.WaitForTopologyId(waitTimeout) + if err != nil { + mc.DumpLogs() + t.Fatalf("cluster did not come back after a restart: %v", err) + } + if after != before { + mc.DumpLogs() + t.Fatalf("-raftBootstrap re-seeded the cluster: TopologyId %s became %s", before, after) + } + + // The leader answers for the whole cluster, so a follower that + // forked is only visible in its own log. + seen, err := mc.WaitForNodeTopologyIds(waitTimeout) + if err != nil { + mc.DumpLogs() + t.Fatal(err) + } + for i, ids := range seen { + for _, id := range ids { + if id != before { + mc.DumpLogs() + t.Fatalf("master %d saw TopologyId %s, want %s", i, id, before) + } + } + if mc.LogContains(i, "Split-brain detected") { + mc.DumpLogs() + t.Fatalf("master %d hit the split-brain guard", i) + } + } + }) + } +} diff --git a/weed/command/master.go b/weed/command/master.go index ebe5b8f0d..87a3cd68a 100644 --- a/weed/command/master.go +++ b/weed/command/master.go @@ -104,7 +104,7 @@ func init() { m.heartbeatInterval = cmdMaster.Flag.Duration("heartbeatInterval", 300*time.Millisecond, "heartbeat interval of master servers, and will be randomly multiplied by [1, 1.25)") m.electionTimeout = cmdMaster.Flag.Duration("electionTimeout", 10*time.Second, "election timeout of master servers") m.raftHashicorp = cmdMaster.Flag.Bool("raftHashicorp", false, "use hashicorp raft") - m.raftBootstrap = cmdMaster.Flag.Bool("raftBootstrap", false, "Whether to bootstrap the Raft cluster") + m.raftBootstrap = cmdMaster.Flag.Bool("raftBootstrap", false, "deprecated and ignored: the first master in -peers mints the Raft cluster on its own once it sees no leader anywhere") m.telemetryUrl = cmdMaster.Flag.String("telemetry.url", "https://telemetry.seaweedfs.com/api/collect", "telemetry server URL to send usage statistics") m.telemetryEnabled = cmdMaster.Flag.Bool("telemetry", true, "report anonymous cluster statistics to telemetry.url, use -telemetry=false to opt out") m.debug = cmdMaster.Flag.Bool("debug", false, "serves runtime profiling data via pprof on the port specified by -debug.port") @@ -211,6 +211,10 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { isSingleMaster := isSingleMasterMode(*masterOption.peers) + if *masterOption.raftBootstrap { + glog.V(0).Infof("-raftBootstrap is ignored: masters mint a cluster on their own when no peer has a leader, and never over existing raft state") + } + raftServerOption := &weed_server.RaftServerOption{ GrpcDialOption: security.LoadClientTLS(util.GetViper(), "grpc.master"), Peers: masterPeers, @@ -221,7 +225,6 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { SingleMaster: isSingleMaster, HeartbeatInterval: *masterOption.heartbeatInterval, ElectionTimeout: *masterOption.electionTimeout, - RaftBootstrap: *masterOption.raftBootstrap, } var raftServer *weed_server.RaftServer var err error @@ -278,8 +281,10 @@ func startMaster(masterOption MasterOptions, masterWhiteList []string) { // 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. + // only when nobody has one does the first peer mint a new cluster. Keeping + // that one peer the sole authority is what stops a partition from minting + // two clusters. 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 diff --git a/weed/server/raft_hashicorp.go b/weed/server/raft_hashicorp.go index 7734fb364..f64c0d26d 100644 --- a/weed/server/raft_hashicorp.go +++ b/weed/server/raft_hashicorp.go @@ -6,7 +6,6 @@ package weed_server import ( "encoding/json" "fmt" - "io" "math/rand/v2" "os" "path" @@ -36,28 +35,6 @@ func raftServerID(server pb.ServerAddress) string { return server.ToHttpAddress() } -// recoverTopologyIdFromHashicorpSnapshot reads the TopologyId from the latest -// hashicorp raft snapshot before state cleanup. -func recoverTopologyIdFromHashicorpSnapshot(dataDir string, topo *topology.Topology) { - fss, err := raft.NewFileSnapshotStore(dataDir, 1, io.Discard) - if err != nil { - return - } - snapshots, err := fss.List() - if err != nil || len(snapshots) == 0 { - return - } - _, rc, err := fss.Open(snapshots[0].ID) - if err != nil { - return - } - defer rc.Close() - - if b, err := io.ReadAll(rc); err == nil { - recoverTopologyIdFromState(b, topo) - } -} - func (s *RaftServer) AddPeersConfiguration() (cfg raft.Configuration) { for _, peer := range s.peers { cfg.Servers = append(cfg.Servers, raft.Server{ @@ -169,13 +146,6 @@ func NewHashicorpRaftServer(option *RaftServerOption) (*RaftServer, error) { return nil, fmt.Errorf("raft.ValidateConfig: %w", err) } - if option.RaftBootstrap { - recoverTopologyIdFromHashicorpSnapshot(s.dataDir, option.Topo) - - os.RemoveAll(path.Join(s.dataDir, ldbFile)) - os.RemoveAll(path.Join(s.dataDir, sdbFile)) - os.RemoveAll(path.Join(s.dataDir, "snapshots")) - } if err := os.MkdirAll(path.Join(s.dataDir, "snapshots"), os.ModePerm); err != nil { return nil, err } @@ -204,16 +174,10 @@ func NewHashicorpRaftServer(option *RaftServerOption) (*RaftServer, error) { return nil, fmt.Errorf("raft.NewRaft: %w", err) } - // 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. + // 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) - } - } go s.monitorLeaderLoop(updatePeers) diff --git a/weed/server/raft_server.go b/weed/server/raft_server.go index ef6799512..55d92254c 100644 --- a/weed/server/raft_server.go +++ b/weed/server/raft_server.go @@ -35,7 +35,6 @@ type RaftServerOption struct { SingleMaster bool HeartbeatInterval time.Duration ElectionTimeout time.Duration - RaftBootstrap bool } type RaftServer struct { @@ -306,7 +305,8 @@ func (s *RaftServer) HasExistingState() bool { // 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. +// never merge. Committed state is never bootstrapped over — hashicorp refuses, +// and goraft callers reach here only past HasExistingState. func (s *RaftServer) Bootstrap() error { glog.V(0).Infoln("Initializing new cluster")