From 4af5447707e922ed6235670f04a7845d7806f757 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Thu, 16 Mar 2023 14:50:25 -0400 Subject: [PATCH] Ensure that all databases are always closed on exit This is really important for file based datastores, and failure to close even a couple will have a highly significant performance impact due to a long-term defra bug, where if the directory hosting defra is deleted whilst a database instance is active the instance will hang/infinate-loop chewing up a huge amount of resources. As the file based directories are deleted by `go test` at the end of each test, this magnifies the effect of any lingering instances. In the case of this issue I believe only a very small number of databases were leaked, yet it nearly doubled the execution time of the change detector (the only potential leaks I found were a couple of panics, and a couple of subscription tests, it could be that as few as 3-4 leaked databases were responsible for the loss of performance). --- tests/integration/utils2.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/integration/utils2.go b/tests/integration/utils2.go index cc305d222c..49a02d8bf0 100644 --- a/tests/integration/utils2.go +++ b/tests/integration/utils2.go @@ -307,6 +307,10 @@ func executeTestCase( syncChans := []chan struct{}{} nodeAddresses := []string{} nodes := getStartingNodes(ctx, t, dbt, collectionNames, testCase) + // It is very important that the databases are always closed, otherwise resources will leak + // as tests run. This is particularly important for file based datastores. + defer closeNodes(ctx, t, &nodes) + // Documents and Collections may already exist in the database if actions have been split // by the change detector so we should fetch them here at the start too (if they exist). // collections are by node (index), as they are specific to nodes. @@ -400,8 +404,15 @@ func executeTestCase( assert.Fail(t, "timeout occurred while waiting for data stream", testCase.Description) } } +} - for _, node := range nodes { +// closeNodes closes all the given nodes, ensuring that resources are properly released. +func closeNodes( + ctx context.Context, + t *testing.T, + nodes *[]*node.Node, +) { + for _, node := range *nodes { if node.Peer != nil { err := node.Close() require.NoError(t, err)