Skip to content

fix(runner): add minimiser close cleanup #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
linkdata wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(runner): add minimiser close cleanup #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,26 @@ func newDnstapMinimiser(logger *slog.Logger, edmConf edmConfiger) (*dnstapMinimi
return edm, nil
}

// Close releases resources held by the minimiser that aren't owned by the
// run lifecycle (notably the fsnotify watcher, which holds an inotify
// instance - a scarce per-user resource on Linux). Production callers go
// through the run loop in Run(), which already defers fsWatcher.Close;
// tests that construct a minimiser without running it must call this
// directly (typically via t.Cleanup) to avoid exhausting
// /proc/sys/fs/inotify/max_user_instances under -count=N.
func (edm *dnstapMinimiser) Close() error {
if edm.stop != nil {
edm.stop()
}
if edm.fsWatcher != nil {
if err := edm.fsWatcher.Close(); err != nil {
return err
}
edm.fsWatcher = nil
}
return nil
}

type wellKnownDomainsTracker struct {
mutex sync.RWMutex
wellKnownDomainsData
Expand Down
14 changes: 14 additions & 0 deletions pkg/runner/runner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ func TestIgnoredClientIPsValid(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

testdataFile1 := "testdata/ignored-client-ips.valid1"
testdataFile2 := "testdata/ignored-client-ips.valid2"
Expand Down Expand Up @@ -407,6 +408,7 @@ func TestIgnoredClientIPsEmptyLinesComments(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

testdataFile := "testdata/ignored-client-ips.empty-lines-and-comments"

Expand Down Expand Up @@ -473,6 +475,7 @@ func TestIgnoredClientIPsEmpty(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

testdataFile := "testdata/ignored-client-ips.valid1"
// To make sure reading an empty file resets stuff as expected first read in a file with content
Expand Down Expand Up @@ -560,6 +563,7 @@ func TestIgnoredClientIPsUnset(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

// To make sure unsetting the filename used for ignored client IPs
// resets stuff as expected first read in a file with content
Expand Down Expand Up @@ -633,6 +637,7 @@ func TestIgnoredClientIPsInvalidClient(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

// Even if we are testing invalid data we still need to have loaded a
// IP file with at least one valid entry in it to even inspect the
Expand Down Expand Up @@ -680,6 +685,7 @@ func TestIgnoredQuestionNamesValid(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

testdataFile1 := "testdata/ignored-question-names.valid1.dawg"
testdataFile2 := "testdata/ignored-question-names.valid2.dawg"
Expand Down Expand Up @@ -831,6 +837,7 @@ func TestIgnoredQuestionNamesEmpty(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

// To make sure reading an empty file resets stuff as expected first read in a file with content
testdataFile := "testdata/ignored-question-names.valid1.dawg"
Expand Down Expand Up @@ -911,6 +918,7 @@ func TestIgnoredQuestionNamesUnset(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

// To make sure unsetting the filename used for ignored question names
// resets stuff as expected first read in a file with content
Expand Down Expand Up @@ -1264,6 +1272,7 @@ func TestPseudonymiseDnstap(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

if edm.cryptopanCache != nil {
if edm.cryptopanCache.Len() != 0 {
Expand Down Expand Up @@ -1559,6 +1568,7 @@ func BenchmarkPseudonymiseDnstapWithCache4(b *testing.B) {
if err != nil {
b.Fatalf("unable to setup edm: %s", err)
}
b.Cleanup(func() { _ = edm.Close() })

b.ResetTimer()
for n := 0; n < b.N; n++ {
Expand Down Expand Up @@ -1591,6 +1601,7 @@ func BenchmarkPseudonymiseDnstapWithoutCache4(b *testing.B) {
if err != nil {
b.Fatalf("unable to setup edm: %s", err)
}
b.Cleanup(func() { _ = edm.Close() })

b.ResetTimer()
for n := 0; n < b.N; n++ {
Expand Down Expand Up @@ -1620,6 +1631,7 @@ func BenchmarkPseudonymiseDnstapWithCache6(b *testing.B) {
if err != nil {
b.Fatalf("unable to setup edm: %s", err)
}
b.Cleanup(func() { _ = edm.Close() })

b.ResetTimer()
for n := 0; n < b.N; n++ {
Expand Down Expand Up @@ -1652,6 +1664,7 @@ func BenchmarkPseudonymiseDnstapWithoutCache6(b *testing.B) {
if err != nil {
b.Fatalf("unable to setup edm: %s", err)
}
b.Cleanup(func() { _ = edm.Close() })

b.ResetTimer()
for n := 0; n < b.N; n++ {
Expand Down Expand Up @@ -2044,6 +2057,7 @@ func TestWriteHistogramParquetExplicitThreshold(t *testing.T) {
if err != nil {
t.Fatalf("unable to setup edm: %s", err)
}
t.Cleanup(func() { _ = edm.Close() })

tests := []struct {
description string
Expand Down