mirror of
https://gitcode.com/JianFeeeee/webui4frpc.git
synced 2026-10-15 12:14:37 +00:00
前两个提交修好分裂与冻结后,滚动重启实测又暴露第三个缺口:
.106 重入成功 → adopt-state(topology 里 .106 是 gitea_ssh 的 owner)
→ 但 .106 没有任何 frpc worker,且日志里从无 reconcile
拓扑说 .106 拥有该转发,进程却不存在 —— 拓扑与实际不一致。
## 根因:代码与自己注释矛盾
启动 reconcile 的注释原文就是「every node (leader or not) checks its OWN topology
entries and re-claims any whose per-forward frpc worker is absent」,但代码第一行是
for { if ring.IsLeader() { break }; sleep 2s }
即必须先当上 leader 才执行。reconcile 做的是**本节点**的活(重建自己拥有的
worker),而三分之一个环里每台都拥有转发 —— 于是所有非 leader 节点永远等不到,
worker 永远不重建。节点可以「已回到集群、拓扑里写着它 own 某转发、却没有任何进程」
这种自相矛盾的状态。
## 修法
新增 Engine.HasSelfInTopology():当前拓扑是否指名本节点为某条 active 转发
的 owner。reconcile 改等这个条件(30s 上限后无论如何继续,保证孤立节点也能推进),
不再等 leadership。领导者与 reconcile 本就无关:follower 也 own 转发。
## 测试
TestHasSelfInTopology:用**两节点**环,转发由非 leader 的 n2 持有,断言非 leader
仍然报告 true。
★ 这条测试第一版是**假绿**:单节点环里唯一的成员必然是 leader,两种实现结果相同,
加回 IsLeader 门控后测试仍全绿(已实测)。改成两节点、明确断言
`precondition: n2 must not be the leader` 之后,加回门控如期变红:
a non-leader that owns a forward reports false — the startup reconcile would
skip it and leave its worker down
go build / go vet / go test ./... 全绿,gofmt 干净。
196 lines
8.3 KiB
Go
196 lines
8.3 KiB
Go
package cluster
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"webui4frpc/internal/store"
|
|
)
|
|
|
|
// staleTokenWith copies the engine's current state but rewrites the given
|
|
// forward's disabled flag, standing in for a token a neighbour captured before
|
|
// this node's decision.
|
|
func staleTokenWith(eng *Engine, local, remote string, port int, disabled bool) State {
|
|
s := eng.state
|
|
out := State{
|
|
LeaderID: s.LeaderID,
|
|
Nodes: s.Nodes,
|
|
Cycle: s.Cycle,
|
|
Topology: map[string]*TopoEntry{},
|
|
}
|
|
for id, e := range s.Topology {
|
|
cp := *e
|
|
if cp.Local.Name == local && cp.Remote.Name == remote && cp.Link.RemotePort == port {
|
|
cp.Link.Disabled = disabled
|
|
cp.Active = !disabled
|
|
}
|
|
out.Topology[id] = &cp
|
|
}
|
|
return out
|
|
}
|
|
|
|
// TestLocalDisableSurvivesAdoption is the regression test for the wipe that made
|
|
// the ring's authority unachievable.
|
|
//
|
|
// OnToken/AdoptState replace e.state wholesale. A stop decided between two token
|
|
// cycles was therefore erased by the next adoption whenever the incoming token
|
|
// had been captured before the decision — so the flag never reached the other
|
|
// members and the forward kept running. Written as a probe BEFORE the fix: it
|
|
// reported "after adopting a stale token: known=true disabled=false".
|
|
func TestLocalDisableSurvivesAdoption(t *testing.T) {
|
|
eng := newTestEngine("n1", true)
|
|
eng.state.AddPending(store.Local{Name: "web"}, store.Remote{Name: "frps1"}, store.Link{RemotePort: 18081})
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 1, State: eng.state, SentAt: 1}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
// Local decision: stop it.
|
|
eng.UpdateTopologyDisabled("web", "frps1", 18081, true)
|
|
if d, _ := eng.TopologyDisabled("web", "frps1", 18081); !d {
|
|
t.Fatal("precondition: the local stop should be visible")
|
|
}
|
|
|
|
// A token captured BEFORE the decision arrives.
|
|
stale := staleTokenWith(eng, "web", "frps1", 18081, false)
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 2, State: stale, SentAt: 2}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
// Assert on the STATE, not the TopologyDisabled() accessor: the accessor
|
|
// short-circuits to the pending local decision, so it would report the stop
|
|
// even when the state about to be forwarded still says "enabled". What
|
|
// matters is that the token this node sends on carries the decision.
|
|
assertForwardedDisabled(t, eng, "web", "frps1", 18081, true)
|
|
}
|
|
|
|
// assertForwardedDisabled checks the disabled value a DOWNSTREAM node would see
|
|
// in the token this engine forwards — the value that actually propagates.
|
|
func assertForwardedDisabled(t *testing.T, eng *Engine, local, remote string, port int, want bool) {
|
|
t.Helper()
|
|
// Model one hop: a fresh engine adopts exactly what this one would send.
|
|
peer := newTestEngine("peer", false)
|
|
peer.AdoptState(eng.state)
|
|
got, known := peer.TopologyDisabled(local, remote, port)
|
|
if !known {
|
|
t.Fatalf("peer learned no entry for %s→%s:%d", local, remote, port)
|
|
}
|
|
if got != want {
|
|
t.Fatalf("the forwarded token carries disabled=%v, want %v — the decision did not "+
|
|
"propagate, so the other members would keep the old state", got, want)
|
|
}
|
|
}
|
|
|
|
// TestLocalReEnableSurvivesAdoption: a re-enable needs the same protection. The
|
|
// value false is easy to overlook — nothing "looks" stopped, yet losing it just
|
|
// as silently strands the forward in the stopped state.
|
|
func TestLocalReEnableSurvivesAdoption(t *testing.T) {
|
|
eng := newTestEngine("n1", true)
|
|
eng.state.AddPending(store.Local{Name: "web"}, store.Remote{Name: "frps1"}, store.Link{RemotePort: 18081})
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 1, State: eng.state, SentAt: 1}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
eng.UpdateTopologyDisabled("web", "frps1", 18081, true)
|
|
eng.UpdateTopologyDisabled("web", "frps1", 18081, false)
|
|
|
|
// A token that still says disabled (captured mid-stop) arrives.
|
|
mid := staleTokenWith(eng, "web", "frps1", 18081, true)
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 2, State: mid, SentAt: 2}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
assertForwardedDisabled(t, eng, "web", "frps1", 18081, false)
|
|
}
|
|
|
|
// TestConfirmedDecisionStopsBeingAsserted: once the ring reports the same value
|
|
// the decision is confirmed everywhere, so this node must stop overriding —
|
|
// otherwise it would fight a later decision made elsewhere.
|
|
func TestConfirmedDecisionStopsBeingAsserted(t *testing.T) {
|
|
eng := newTestEngine("n1", true)
|
|
eng.state.AddPending(store.Local{Name: "web"}, store.Remote{Name: "frps1"}, store.Link{RemotePort: 18081})
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 1, State: eng.state, SentAt: 1}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
eng.UpdateTopologyDisabled("web", "frps1", 18081, true)
|
|
if len(eng.localDisabled) != 1 {
|
|
t.Fatalf("a pending decision should be tracked, got %v", eng.localDisabled)
|
|
}
|
|
// The ring comes back agreeing.
|
|
agreed := staleTokenWith(eng, "web", "frps1", 18081, true)
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 2, State: agreed, SentAt: 2}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(eng.localDisabled) != 0 {
|
|
t.Fatalf("a confirmed decision must stop being asserted, still tracking %v", eng.localDisabled)
|
|
}
|
|
|
|
// A peer now decides the opposite; this node must follow the ring.
|
|
peerSaysEnabled := staleTokenWith(eng, "web", "frps1", 18081, false)
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 3, State: peerSaysEnabled, SentAt: 3}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if d, _ := eng.TopologyDisabled("web", "frps1", 18081); d {
|
|
t.Fatal("this node kept overriding a peer's later decision; the ring is not authoritative")
|
|
}
|
|
}
|
|
|
|
// TestLocalDecisionsDoNotAccumulate: the tracking map is swept against the live
|
|
// topology, so a long-lived cluster cannot grow it without bound.
|
|
func TestLocalDecisionsDoNotAccumulate(t *testing.T) {
|
|
eng := newTestEngine("n1", true)
|
|
eng.state.AddPending(store.Local{Name: "web"}, store.Remote{Name: "frps1"}, store.Link{RemotePort: 18081})
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 1, State: eng.state, SentAt: 1}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
// Decisions for forwards this node has never seen.
|
|
eng.UpdateTopologyDisabled("ghost1", "frps1", 1, true)
|
|
eng.UpdateTopologyDisabled("ghost2", "frps1", 2, true)
|
|
if len(eng.localDisabled) != 2 {
|
|
t.Fatalf("expected 2 tracked decisions, got %d", len(eng.localDisabled))
|
|
}
|
|
// A cycle runs: neither ghost is in the topology, so both are swept.
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 2, State: eng.state, SentAt: 2}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(eng.localDisabled) != 0 {
|
|
t.Fatalf("decisions for forwards outside the topology must be swept, got %v", eng.localDisabled)
|
|
}
|
|
}
|
|
|
|
// TestHasSelfInTopology is the wait condition for the startup reconcile: the
|
|
// reconcile rebuilds the workers for forwards THIS node owns, so a node can only
|
|
// act once the adopted state actually names it as an owner.
|
|
//
|
|
// It is deliberately independent of leadership — the bug it guards against was a
|
|
// reconcile that waited for leadership and therefore never ran on followers, even
|
|
// though followers own forwards.
|
|
func TestHasSelfInTopology(t *testing.T) {
|
|
// Two nodes; the forward is owned by n2, which is NOT the leader. A single-node
|
|
// ring would make this test pass under either implementation, because the only
|
|
// member is necessarily the leader — which is exactly how the first version of
|
|
// this test came to be a false green.
|
|
eng := newTestEngine("n2", false)
|
|
eng.state.AddPending(store.Local{Name: "web"}, store.Remote{Name: "frps1"}, store.Link{RemotePort: 18081})
|
|
if _, err := eng.OnToken(context.Background(), &Token{Cycle: 1, State: eng.state, SentAt: 1}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if eng.IsLeader() {
|
|
t.Fatal("precondition: n2 must not be the leader in this test")
|
|
}
|
|
// n2 claimed it, so it owns a forward — despite not leading.
|
|
if !eng.HasSelfInTopology() {
|
|
t.Error("a non-leader that owns a forward reports false — the startup reconcile " +
|
|
"would skip it and leave its worker down")
|
|
}
|
|
|
|
// A node that owns nothing must report false (so it does not act early on a
|
|
// stale or empty topology).
|
|
other := newTestEngine("n9", true)
|
|
if other.HasSelfInTopology() {
|
|
t.Error("a node owning nothing must not claim it appears in the topology")
|
|
}
|
|
// Guard against a regression where leadership is folded into the predicate.
|
|
if !eng.HasSelfInTopology() && eng.IsLeader() {
|
|
t.Error("predicate must not be gated on leadership")
|
|
}
|
|
}
|