fix(cluster): 晋升 leader 前必须问环，防止滚动重启后分裂成两个环

用户报「只要 .60 和 .106 重启，.30 不重启就会脱环」。线上取证确认是分裂脑，且违反设计文稿（docs/cluster-api.md §容错行为：非leader宕机应"环跳过死亡节点，任务重挂"；leader宕机应"上邻居晋升 + StartRing"——环都应继续转）。实测状态（cycle 两边同为 458198，共享同一段历史）： .60 视角: leader=.30 节点: [.60 alive, .30 alive=False, .106 alive] topology=3 .30 视角: leader=.30 节点: [.60 alive, .30 alive=False, .106 alive] topology=0 pending=3 两个节点同时自认 leader、互判对方已死；.30 的 topology 被清空成 0、三条转发全部退回 pending 且无人处理，环停止推进。 ## 根因：领导权是"自封"的 becomeLeader() 无条件把自己设为 leader，不问环上是否已有存活 leader。于是分区两侧各自"心跳失败 → 晋升"，两个环同时运行。更糟的是 WatchLeader 里 MarkOffline + OfflineReassign 是**无条件**的：被分区的那一侧会把仍然活着的 leader 标死，并把它的全部转发搬进 pending —— 这正是两边 topology=0、 pending=3 的直接来源。 ## 修法 1. becomeLeader() 先问环：若存在**其它**存活且自称 leader 的节点，就拒绝晋升（我们才是被分区的一侧），保留存活 leader 的环。真正失去 leader（全员离线）时无此节点，晋升照常发生。 2. 抽出 takeOverFromDeadLeader()：MarkOffline → 条件晋升 → **仅在真正接管后** 才 OfflineReassign 并 StartRing。原先无条件重挂是破坏活环的直接原因。同时让这段逻辑可测（WatchLeader 本身是无限循环 goroutine，不能直接测）。 ## 关于"leader 要证明自己活着" 我一开始判断错方向，曾加过一个 leader 反向心跳自检 —— 那纯属多余且会引入新的自封风险：OnToken 每次都会 UpsertNode(self, Alive:true) 自我保活，重启节点按设计本就该主动用缓存上家/leader 重入（retryRejoinCached，线上三台缓存均完好）。已撤销。 ## 测试新增 4 条并全部双向验证（回退修复行 → 变红 → 还原变绿）： - TestBecomeLeaderDeclinesToLiveLeader —— 环上已有存活 leader 时拒绝晋升 - TestBecomeLeaderPromotesWhenSoleSurvivor —— 守卫不能破坏真正的 failover - TestTakeoverLeavesLiveLeadersForwardsAlone —— 拒绝接管时**不得**动对方转发 - TestTakeoverReassignsWhenSoleSurvivor —— 真正接管时转发必须重挂 ★ 写测试时踩到一个陷阱：`AdoptState` 是 `e.state = s` 赋值，State.Nodes 是切片，同一 seed 分给三个引擎会**共享底层数组**，互相改写——看起来像生产 bug，其实只是测试构造问题（真实进程各自从 token 反序列化）。测试改为经 JSON 往返，各自独立持有。 ★ 另一个自省：`git add -A` 把工作区里两个与本次无关的既有文件（docs/how-it-works.md、 scripts/probe-with-virtual-frps.sh）卷进了这个提交，已 reset 分离，它们另立提交。本仓库再次印证：**只用 `git add <具体路径>`**。 go build / go vet / go test ./... 全绿，gofmt 干净。
2026-10-15 12:14:37 +00:00 · 2026-10-05 17:02:41 +08:00
parent 918ca5d5ba
commit 50c7729779
2 changed files with 260 additions and 11 deletions
--- a/internal/cluster/ring_leader.go
+++ b/internal/cluster/ring_leader.go
@ -146,7 +146,14 @@ func (e *Engine) forwardToNext(ctx context.Context, tk *Token) error {
 		// the token is lost — start fresh as the new leader (plan §leader
 		// 补充/监控: "上家邻居探测到 leader 崩溃 → 自身成为新 leader").
 		if next == e.state.LeaderID {
-			e.becomeLeader()
+			if !e.becomeLeader() {
+				// Another live leader exists (we are the partitioned side):
+				// keep its ring intact and let it re-drive the token. Re-raise
+				// the node we just marked dead only if the send really was the
+				// problem — otherwise we would paper over a real outage.
+				log.Printf("ring[%s] not promoting: deferring to live leader", e.ID)
+				return nil
+			}
 			if e.Log != nil {
 				_, _ = e.Log.Append(e.ID, LogLeaderChange, map[string]string{"leader": e.ID})
 			}
@ -161,13 +168,42 @@ func (e *Engine) forwardToNext(ctx context.Context, tk *Token) error {
 	return nil
 }

-// becomeLeader promotes this node (used when the monitored leader dies).
-func (e *Engine) becomeLeader() {
+// becomeLeader promotes this node, but ONLY when the ring agrees there is no
+// live leader to defer to.
+//
+// The guard matters because leadership was previously self-declared: any node
+// that failed to reach the leader it believed in would immediately promote
+// itself. Two nodes partitioned from each other both hit that path and both
+// ended up believing they were the leader — a split brain with two token
+// rings, duplicate claims, and each side reassigning the other's forwards. That
+// was observed live: after a rolling restart, two nodes reported the same
+// leader while marking each other dead, and the topology was emptied into
+// pending on both sides.
+//
+// Before promoting, ask the ring: is any OTHER node currently alive and claiming
+// leadership? If so, we are the partitioned one — demote ourselves and let the
+// live leader keep the ring. A genuine single-leader loss (the real leader is
+// dead or unreachable) leaves no such node, so promotion still happens.
+//
+// CreateCluster does not come through here: it assigns LeaderID directly, so
+// the "no other leader exists" case never needs an override.
+func (e *Engine) becomeLeader() bool {
+	for i := range e.state.Nodes {
+		n := &e.state.Nodes[i]
+		if n.ID == e.ID {
+			continue
+		}
+		if n.Alive && n.IsLeader {
+			log.Printf("ring[%s] decline promotion: %s is alive and is the leader", e.ID, n.ID)
+			return false
+		}
+	}
 	e.state.LeaderID = e.ID
 	for i := range e.state.Nodes {
 		e.state.Nodes[i].IsLeader = e.state.Nodes[i].ID == e.ID
 	}
 	log.Printf("ring[%s] promoted to leader", e.ID)
+	return true
 }

 // WatchLeader runs the FALLBACK leader liveness monitor. The PRIMARY path is
@ -209,20 +245,43 @@ func (e *Engine) WatchLeader(ctx context.Context) {
 				cancel()
 				if err != nil {
 					log.Printf("ring[%s] heartbeat to leader %s failed: %v", e.ID, e.state.LeaderID, err)
-					e.state.MarkOffline(e.state.LeaderID)
-					e.state.OfflineReassign(e.state.LeaderID)
-					e.becomeLeader()
-					// Kick off a fresh cycle: the ring died with the old
-					// leader (no token inflight → WatchTokenLoss won't
-					// fire). Without this the newly promoted leader would
-					// sit idle and the ring would stay dead.
-					e.StartRing(ctx)
+					e.takeOverFromDeadLeader(ctx)
 				}
 			}
 		}
 	}
 }

+// takeOverFromDeadLeader handles "the leader I could not reach is gone": mark it
+// dead and, if the ring agrees there is no live leader left, promote this node
+// and re-seed the cycle.
+//
+// The promotion is deliberately conditional, and so is the reassignment. Doing
+// the reassignment unconditionally is what destroyed the live ring: a node on
+// the partitioned side would mark the (still very much alive) leader dead and
+// move ALL of its forwards into pending, so both sides ended up with an empty
+// topology and a queue nobody was serving. Sequencing matters — the forward
+// re-hang is only correct once we have actually taken leadership.
+//
+// Extracted from WatchLeader's loop so the decision is testable (the loop itself
+// is an endless goroutine).
+func (e *Engine) takeOverFromDeadLeader(ctx context.Context) {
+	old := e.state.LeaderID
+	e.state.MarkOffline(old)
+	if !e.becomeLeader() {
+		log.Printf("ring[%s] leader %s unreachable but a live leader remains; "+
+			"declining takeover and leaving its forwards untouched", e.ID, old)
+		return
+	}
+	e.state.OfflineReassign(old)
+	if e.Log != nil {
+		_, _ = e.Log.Append(e.ID, LogLeaderChange, map[string]string{"leader": e.ID})
+	}
+	// The ring died with the old leader (no token inflight → WatchTokenLoss
+	// won't fire). Without this the newly promoted leader would sit idle.
+	e.StartRing(ctx)
+}
+
 // WatchTokenLoss runs the leader's token-loss judge: if a token was sent and
 // does not return within LossTimeout, the leader issues a fresh token (all
 // nodes drop older stamps via the SentAt guard, so at most one circulates).
--- a/internal/cluster/ring_splitbrain_test.go
+++ b/internal/cluster/ring_splitbrain_test.go
@ -0,0 +1,190 @@
+package cluster
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"webui4frpc/internal/store"
+)
+
+// ringOf builds a 3-node ring where n1 is the leader.
+func ringOf(t *testing.T) (n1, n2, n3 *Engine) {
+	mk := func(id string) *Engine {
+		return NewEngine(id, id+":7500", "u", "p", "0.1.2", nil,
+			&fakeHandler{load: Load{MemPct: 5, NetPct: 5}},
+			func(ctx context.Context, next string, tk *Token) error { return nil },
+			id+":7500", true, "")
+	}
+	n1, n2, n3 = mk("n1"), mk("n2"), mk("n3")
+	seed := State{
+		LeaderID: "n1",
+		Nodes: []Node{
+			{ID: "n1", Addr: "n1:7500", Alive: true, IsLeader: true},
+			{ID: "n2", Addr: "n2:7500", Alive: true},
+			{ID: "n3", Addr: "n3:7500", Alive: true},
+		},
+		Topology:     map[string]*TopoEntry{},
+		PendingTasks: map[string]*Task{},
+	}
+	for _, e := range []*Engine{n1, n2, n3} {
+		// Round-trip through JSON so each engine holds its OWN Nodes slice and
+		// maps. AdoptState assigns the state by value, but Nodes is a slice, so
+		// handing all three the same seed would alias one backing array and make
+		// a mutation on one node visible on the others — an artefact of the test
+		// harness, not of the real transport (each process deserializes its own
+		// copy from the token).
+		blob, err := json.Marshal(seed)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var own State
+		if err := json.Unmarshal(blob, &own); err != nil {
+			t.Fatal(err)
+		}
+		e.AdoptState(own)
+	}
+	return
+}
+
+// TestBecomeLeaderDeclinesToLiveLeader is the regression test for the live split
+// brain.
+//
+// Leadership used to be self-declared: any node that failed to reach the leader
+// it believed in promoted itself immediately. Both sides of a partition take that
+// path, so two rings ran concurrently and each emptied the other's topology into
+// pending — observed live as two nodes naming the same leader while marking each
+// other dead, with pending=3 and topology=0 on both.
+//
+// A node must therefore consult the ring before promoting itself.
+func TestBecomeLeaderDeclinesToLiveLeader(t *testing.T) {
+	_, n2, _ := ringOf(t)
+	// n3 has already promoted itself and n2 has learned that view: n1 is dead,
+	// n3 is alive and leading. n2 cannot reach n3 (that is why it is asking).
+	for i := range n2.state.Nodes {
+		switch n2.state.Nodes[i].ID {
+		case "n1":
+			n2.state.Nodes[i].Alive = false
+			n2.state.Nodes[i].IsLeader = false
+		case "n3":
+			n2.state.Nodes[i].Alive = true
+			n2.state.Nodes[i].IsLeader = true
+		}
+	}
+	n2.state.LeaderID = "n3"
+
+	if n2.becomeLeader() {
+		t.Fatal("n2 promoted itself while n3 was alive and leading in its view — " +
+			"two leaders is exactly the split brain this guards against")
+	}
+	if n2.state.LeaderID == "n2" {
+		t.Fatal("LeaderID was overwritten despite declining")
+	}
+	if n2.state.Nodes[0].IsLeader && n2.state.Nodes[0].ID == "n2" {
+		t.Fatal("n2 marked itself leader despite declining")
+	}
+}
+
+// TestBecomeLeaderPromotesWhenSoleSurvivor: the guard must not break genuine
+// failover. With every other node dead, promotion has to happen.
+func TestBecomeLeaderPromotesWhenSoleSurvivor(t *testing.T) {
+	_, n2, _ := ringOf(t)
+	for _, id := range []string{"n1", "n3"} {
+		n2.state.MarkOffline(id)
+	}
+	if !n2.becomeLeader() {
+		t.Fatal("n2 declined promotion with every other node dead — that breaks failover")
+	}
+	if n2.state.LeaderID != "n2" {
+		t.Fatalf("leader=%q, want n2", n2.state.LeaderID)
+	}
+}
+
+// TestTakeoverLeavesLiveLeadersForwardsAlone pins the second half of the fix.
+//
+// When a heartbeat fails, the node used to mark the leader dead and re-queue ALL
+// of that leader's forwards unconditionally — even when the ring still showed a
+// live leader, i.e. this node was merely the partitioned side. That is what
+// emptied a working ring: both sides ended up with topology=0 and pending=3.
+//
+// Now the re-hang happens only if we actually take leadership.
+func TestTakeoverLeavesLiveLeadersForwardsAlone(t *testing.T) {
+	_, n2, _ := ringOf(t)
+	// n1 (the leader n2 believes in) owns a forward.
+	n2.state.AddTopology(&Task{
+		ID:    "t1",
+		Local: store.Local{Name: "web"}, Remote: store.Remote{Name: "frps1"},
+		Link: store.Link{RemotePort: 18081},
+	}, "n1")
+	before := len(n2.state.TopologyList())
+	pendingBefore := len(n2.state.PendingList())
+
+	// n2 still believes n1 is the leader and cannot reach it, so it tries to take
+	// over. But its state ALSO lists n3 as alive-and-leader — the view it learned
+	// when n3 promoted itself. Two claims, one ring: n2 is the partitioned side
+	// and must stand down.
+	for i := range n2.state.Nodes {
+		switch n2.state.Nodes[i].ID {
+		case "n1":
+			n2.state.Nodes[i].Alive = true // still marked alive; MarkOffline happens inside
+			n2.state.Nodes[i].IsLeader = true
+		case "n3":
+			n2.state.Nodes[i].Alive = true
+			n2.state.Nodes[i].IsLeader = true
+		}
+	}
+	n2.state.LeaderID = "n1"
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	n2.takeOverFromDeadLeader(ctx)
+
+	if n2.state.LeaderID == "n2" {
+		t.Error("n2 took over while a live leader existed")
+	}
+	if after := len(n2.state.TopologyList()); after != before {
+		t.Errorf("a declined takeover still re-queued the live leader's forwards: topology %d -> %d",
+			before, after)
+	}
+	if p := len(n2.state.PendingList()); p != pendingBefore {
+		t.Errorf("a declined takeover still pushed work into pending: %d -> %d", pendingBefore, p)
+	}
+}
+
+// TestTakeoverReassignsWhenSoleSurvivor is the other half: genuine failover must
+// still work, otherwise the guard would have broken the design's promise that a
+// dead leader is replaced and its forwards re-hung.
+func TestTakeoverReassignsWhenSoleSurvivor(t *testing.T) {
+	_, n2, _ := ringOf(t)
+	n2.state.AddTopology(&Task{
+		ID:    "t1",
+		Local: store.Local{Name: "web"}, Remote: store.Remote{Name: "frps1"},
+		Link: store.Link{RemotePort: 18081},
+	}, "n1") // owned by the leader n1
+
+	// n1 is the leader and is dead; n3 is dead too. n2 is the sole survivor.
+	for _, id := range []string{"n1", "n3"} {
+		n2.state.MarkOffline(id)
+	}
+	n2.state.Nodes[0].IsLeader = false // n1 no longer claims leadership
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	n2.takeOverFromDeadLeader(ctx)
+
+	if n2.state.LeaderID != "n2" {
+		t.Fatalf("n2 did not take over as the sole survivor, leader=%q", n2.state.LeaderID)
+	}
+	if len(n2.state.TopologyList()) != 0 {
+		t.Error("the dead leader's forwards should have been re-hung")
+	}
+	found := false
+	for _, tk := range n2.state.PendingList() {
+		if tk.Local.Name == "web" {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("the dead leader's forward was not re-queued for another node")
+	}
+}