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) + } +}