Skip to content

Commit

Permalink
Fix goroutine leaks raised by plugin/storage/integration/cassandra_te…
Browse files Browse the repository at this point in the history
…st.go (#5311)

## Which problem is this PR solving?
- Solves part of #5006

## Description of the changes
- Fixes goroutine leaks in the shutdown of cassandra Factory
- It does not add the goleak detection because there are still failures
in this package related to other components (e.g. grpc, elasticsearch)

## How was this change tested?
- Add a goleak detection (i.e. this file
105a4d7)
- `STORAGE=cassandra go test -v  ./plugin/storage/integration`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Will Sewell <[email protected]>
  • Loading branch information
Will Sewell authored Apr 8, 2024
1 parent ea53d8e commit 6a26005
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
14 changes: 11 additions & 3 deletions plugin/storage/cassandra/factory.go
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()
}
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
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
3 changes: 3 additions & 0 deletions plugin/storage/integration/cassandra_test.go
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 Down

0 comments on commit 6a26005

Please sign in to comment.