From bc49c1a68f574e12a770a711efb9bc184bfa0a06 Mon Sep 17 00:00:00 2001 From: Sanaei Date: Tue, 15 Sep 2026 20:39:32 +0200 Subject: [PATCH] fix(node): release a deleted node's metric series and HTTP client Deleting a node must free what the master keeps per node in memory. Delete dropped the node's cpu and mem series but not netUp and netDown, which the heartbeat records too, so each deleted node leaked two tiered histories. It now drops every NodeMetricKeys entry. InvalidateNode, called on node edit, disable and delete, cleared only the cached Remote. The pooled HTTP client and its transport stayed cached until a later call for the same node pruned them, which a deleted node never makes. InvalidateNode now drops those too, outside the manager lock; an edited node pays one fresh handshake on its next call. --- internal/web/runtime/manager.go | 7 ++- .../web/runtime/tls_client_invalidate_test.go | 25 +++++++++++ internal/web/service/node.go | 5 ++- .../web/service/node_delete_cleanup_test.go | 43 +++++++++++++++++++ 4 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 internal/web/runtime/tls_client_invalidate_test.go create mode 100644 internal/web/service/node_delete_cleanup_test.go diff --git a/internal/web/runtime/manager.go b/internal/web/runtime/manager.go index ff6afc2fd..a286b28a8 100644 --- a/internal/web/runtime/manager.go +++ b/internal/web/runtime/manager.go @@ -168,10 +168,15 @@ func sameRemoteIdentity(a, b *model.Node) bool { a.OutboundTag == b.OutboundTag } +// InvalidateNode forgets everything cached for a node, its pooled HTTP client too: +// only a later call for that node would prune it, and a deleted node never makes one. func (m *Manager) InvalidateNode(nodeID int) { m.mu.Lock() - defer m.mu.Unlock() delete(m.remotes, nodeID) + m.mu.Unlock() + nodeClientsMu.Lock() + dropNodeClients(nodeID, "") + nodeClientsMu.Unlock() } func loadNode(id int) (*model.Node, error) { diff --git a/internal/web/runtime/tls_client_invalidate_test.go b/internal/web/runtime/tls_client_invalidate_test.go new file mode 100644 index 000000000..f34c8bf99 --- /dev/null +++ b/internal/web/runtime/tls_client_invalidate_test.go @@ -0,0 +1,25 @@ +package runtime + +import ( + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// Only a later call for the same node pruned its pooled client, so a deleted +// node kept its client and transport cached for the life of the process. +func TestInvalidateNodeDropsItsCachedHTTPClient(t *testing.T) { + node := &model.Node{Id: 8801, Address: "node.example.test", Port: 443, Scheme: "https", TlsVerifyMode: "skip"} + if _, err := HTTPClientForNode(node, ""); err != nil { + t.Fatalf("HTTPClientForNode: %v", err) + } + if got := nodeClientEntries(node.Id); got != 1 { + t.Fatalf("cached clients before invalidation = %d, want 1", got) + } + + NewManager(LocalDeps{}).InvalidateNode(node.Id) + + if got := nodeClientEntries(node.Id); got != 0 { + t.Fatalf("cached clients after InvalidateNode = %d, want 0", got) + } +} diff --git a/internal/web/service/node.go b/internal/web/service/node.go index bddf9bb30..b2f48aec6 100644 --- a/internal/web/service/node.go +++ b/internal/web/service/node.go @@ -878,8 +878,9 @@ func (s *NodeService) Delete(id int) error { if mgr := runtime.GetManager(); mgr != nil { mgr.InvalidateNode(id) } - nodeMetrics.drop(nodeMetricKey(id, "cpu")) - nodeMetrics.drop(nodeMetricKey(id, "mem")) + for _, metric := range NodeMetricKeys { + nodeMetrics.drop(nodeMetricKey(id, metric)) + } return nil } diff --git a/internal/web/service/node_delete_cleanup_test.go b/internal/web/service/node_delete_cleanup_test.go new file mode 100644 index 000000000..3bfa6ca14 --- /dev/null +++ b/internal/web/service/node_delete_cleanup_test.go @@ -0,0 +1,43 @@ +package service + +import ( + "strings" + "testing" + "time" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// Deleting a node dropped only its cpu and mem series, while the heartbeat also +// records netUp and netDown, so each deleted node leaked two histories for good. +func TestDeleteNodeDropsEveryMetricSeries(t *testing.T) { + setupConflictDB(t) + node := &model.Node{Id: 9101, Name: "gone", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true, Status: "online"} + if err := database.GetDB().Create(node).Error; err != nil { + t.Fatalf("create node: %v", err) + } + ns := NodeService{} + if err := ns.UpdateHeartbeat(node.Id, HeartbeatPatch{ + Status: "online", LastHeartbeat: time.Now().Unix(), CpuPct: 1, MemPct: 2, NetUp: 3, NetDown: 4, + }); err != nil { + t.Fatalf("UpdateHeartbeat: %v", err) + } + + if err := ns.Delete(node.Id); err != nil { + t.Fatalf("Delete: %v", err) + } + + prefix := nodeMetricKey(node.Id, "") + var left []string + nodeMetrics.mu.Lock() + for key := range nodeMetrics.series { + if strings.HasPrefix(key, prefix) { + left = append(left, key) + } + } + nodeMetrics.mu.Unlock() + if len(left) != 0 { + t.Fatalf("metric series left after deleting the node: %v", left) + } +}