From 8e16b880c8d161e6174ec7da712ca77b2c64a19a Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sat, 31 Oct 2026 01:55:42 +0000 Subject: [PATCH] Close the cluster when the server stops Server.stop never closed the cluster, so a stopped node stayed registered and its heartbeat kept running against the closed database, logging from leaked goroutines. In CI that raced with captureLog's unsynchronized buffer in a later test. captureLog now uses syncBuffer as well. --- server/server.go | 3 +++ server/server_admin_test.go | 4 ++-- server/server_cluster_test.go | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/server/server.go b/server/server.go index ccd8af6c..9111fbec 100644 --- a/server/server.go +++ b/server/server.go @@ -617,6 +617,9 @@ func (s *Server) stop() { if s.tail != nil { s.tail.Close() // Before the databases close under it } + if s.cluster != nil { + s.cluster.Close() // Before the databases close, so it can deregister the node and its heartbeat stops + } s.closeDatabases() if s.ban != nil { s.ban.Close() diff --git a/server/server_admin_test.go b/server/server_admin_test.go index 9378f7bd..40362c0d 100644 --- a/server/server_admin_test.go +++ b/server/server_admin_test.go @@ -1577,8 +1577,8 @@ func (b *syncBuffer) String() string { } func captureLog(t *testing.T, f func()) string { - var buf bytes.Buffer - log.SetOutput(&buf) + buf := &syncBuffer{} // Background goroutines of other tests' servers may log meanwhile, too + log.SetOutput(buf) log.SetLevel(log.InfoLevel) defer func() { log.SetLevel(log.ErrorLevel) // As in TestMain diff --git a/server/server_cluster_test.go b/server/server_cluster_test.go index 47fe9804..c3e4b08e 100644 --- a/server/server_cluster_test.go +++ b/server/server_cluster_test.go @@ -237,6 +237,27 @@ func TestServer_Cluster_Role(t *testing.T) { require.Equal(t, cluster.RolePassive, members[0].Role) } +func TestServer_Cluster_StopClosesCluster(t *testing.T) { + // Stopping the server must stop the cluster: it deregisters the node, so a node starting + // after it no longer sees it, and its heartbeat does not outlive the server's database + schemaDSN := dbtest.CreateTestPostgresSchema(t) + newNode := func(nodeID string) *Server { + conf := newTestConfig(t, schemaDSN) + conf.ClusterNodeID = nodeID + conf.ClusterListen = "127.0.0.1:1" // Enables clustering; not bound since Run() is not called + conf.ClusterSecret = "s3cret" + conf.ClusterAdvertiseURL = "http://127.0.0.1:1" + return newTestServer(t, conf) + } + sA := newNode("node-a") + require.Len(t, sA.cluster.Members(), 1) + sA.Stop() + sB := newNode("node-b") + members := sB.cluster.Members() + require.Len(t, members, 1) + require.Equal(t, cluster.NodeID("node-b"), members[0].NodeID) +} + func TestServer_Cluster_ActiveWithPassivePeer(t *testing.T) { // The setup for trying clustering beside production: the production server is the only // active node, a second one runs passive. The active one leads right away, both deliver