Skip to content
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

Fix goroutine leaks raised by plugin/storage/integration/cassandra_test.go #5311

Merged
merged 9 commits into from
Apr 8, 2024
Merged

Fix goroutine leaks raised by plugin/storage/integration/cassandra_test.go #5311

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
105a4d7
Add goroutine leak checks to plugin/storage/integration
Mar 29, 2024
55089ae
Fix goroutine leaks in integration/cassandra_test.go
Mar 31, 2024
9914337
Merge branch 'main' into add-goleak-to-plugin-storage-integration
Apr 1, 2024
a56c63a
Remove unnecessary line
Apr 8, 2024
1202e98
Don't swallow errors in cassandra Factory Close method
Apr 8, 2024
6013b8d
Stop testing for goroutine leaks in plugin/storage/integration
Apr 8, 2024
5d222e0
Mock the cassandra Factor Close method
Apr 8, 2024
0970720
Simplify cleanup of cassandra Factory in tests
Apr 8, 2024
0542a88
Remove accidentally commented out line
Apr 8, 2024
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
14 changes: 11 additions & 3 deletions plugin/storage/cassandra/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,19 @@ var _ io.Closer = (*Factory)(nil)

// Close closes the resources held by the factory
func (f *Factory) Close() error {
f.Options.Get(archiveStorageConfig)
if f.primarySession != nil {
f.primarySession.Close()
}
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
if f.archiveSession != nil {
f.archiveSession.Close()
}

var errs []error
if cfg := f.Options.Get(archiveStorageConfig); cfg != nil {
cfg.TLS.Close()
errs = append(errs, cfg.TLS.Close())
}
return f.Options.GetPrimary().TLS.Close()
errs = append(errs, f.Options.GetPrimary().TLS.Close())
return errors.Join(errs...)
}

// PrimarySession is used from integration tests to clean database between tests
Expand Down
1 change: 1 addition & 0 deletions plugin/storage/cassandra/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func TestCassandraFactory(t *testing.T) {
query = &mocks.Query{}
)
session.On("Query", mock.AnythingOfType("string"), mock.Anything).Return(query)
session.On("Close").Return()
query.On("Exec").Return(nil)
f.primaryConfig = newMockSessionBuilder(session, nil)
f.archiveConfig = newMockSessionBuilder(nil, errors.New("made-up error"))
Expand Down
5 changes: 4 additions & 1 deletion plugin/storage/integration/cassandra_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func (s *CassandraStorageIntegration) initializeCassandra(t *testing.T) {
s.SamplingStore, err = f.CreateSamplingStore(0)
require.NoError(t, err)
s.initializeDependencyReaderAndWriter(t, f)
t.Cleanup(func() {
require.NoError(t, f.Close())
})
}

func (s *CassandraStorageIntegration) initializeDependencyReaderAndWriter(t *testing.T, f *cassandra.Factory) {
Expand All @@ -109,7 +112,7 @@ func (s *CassandraStorageIntegration) initializeDependencyReaderAndWriter(t *tes
}

func TestCassandraStorage(t *testing.T) {
skipUnlessEnv(t, "cassandra")
// skipUnlessEnv(t, "cassandra")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to revert this

s := newCassandraStorageIntegration()
s.initializeCassandra(t)
s.RunAll(t)
Expand Down
Loading