Skip to content

Conversation

@pawelchcki
Copy link
Contributor

@pawelchcki pawelchcki commented Jul 4, 2018

In multithreaded setup and when using connection pool while performing many SQL queries, the active_span in finishing of event was not always the same to span created at the start of an event.

This caused some odd spans to not properly close - leaking memory in the process. Benchmark application memory use did baloon 2-4x in a matter of minutes

This PR addresses this issue.

Its possible it might one of the causes of #431, as this regression was introduced in 0.12.0 and affects multithreaded apps - i.e. Sidekiq.

Todo:

  • fix tests to align with new implementation

@pawelchcki pawelchcki added the do-not-merge/WIP Not ready for merge label Jul 4, 2018
@pawelchcki pawelchcki changed the title Avoid leaking memory by not closing all created spans Avoid leaking memory by closing all created spans Jul 4, 2018
@pawelchcki pawelchcki changed the title Avoid leaking memory by closing all created spans Make sure spans are closed when processing notifications Jul 9, 2018
@pawelchcki pawelchcki added bug Involves a bug core Involves Datadog core libraries integrations Involves tracing integrations and removed do-not-merge/WIP Not ready for merge labels Jul 9, 2018
end

it 'sets span in payload' do
expect { subject }.to change { payload[:datadog_span] }.to be_instance_of(Datadog::Span)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@delner delner self-requested a review July 11, 2018 17:09
@delner delner added this to the 0.13.1 milestone Jul 13, 2018
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

👍

@pawelchcki pawelchcki merged commit 40024f8 into master Jul 17, 2018
@delner delner deleted the bugfix/avoid_leaking_memory_by_not_closing_spans branch July 17, 2018 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Involves a bug core Involves Datadog core libraries integrations Involves tracing integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants