Skip to content

Check goroutines in places related to recent found leaks#12106

Merged
jsoriano merged 20 commits intoelastic:masterfrom
jsoriano:test-close-on-signal
May 24, 2019
Merged

Check goroutines in places related to recent found leaks#12106
jsoriano merged 20 commits intoelastic:masterfrom
jsoriano:test-close-on-signal

Conversation

@jsoriano
Copy link
Copy Markdown
Member

@jsoriano jsoriano commented May 8, 2019

Follows the strategy used in #10850 to check for goroutine leaks
on tests in some places related to the leaks investigated as part
of #9302.

Several things here:

@jsoriano jsoriano added bug Filebeat Filebeat libbeat :Testing Team:Integrations Label for the Integrations team labels May 8, 2019
@jsoriano jsoriano requested a review from a team as a code owner May 8, 2019 19:37
@jsoriano jsoriano self-assigned this May 8, 2019
@jsoriano jsoriano requested a review from exekias May 8, 2019 19:47
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

Do our runtime metrics that get logged every 30s include a gauge for the number of goroutines? That would be helpful for identifying if a leak is introduced to a Beat.

@jsoriano
Copy link
Copy Markdown
Member Author

jsoriano commented May 9, 2019

I am moving the fix for the leak in NewInput to its own PR (#12125), so we can continue discussing about these tests here without blocking this fix.

@jsoriano
Copy link
Copy Markdown
Member Author

jsoriano commented May 9, 2019

Do our runtime metrics that get logged every 30s include a gauge for the number of goroutines? That would be helpful for identifying if a leak is introduced to a Beat.

Created PR for that #12135

@jsoriano jsoriano force-pushed the test-close-on-signal branch from b8c4367 to 1b42a6b Compare May 10, 2019 12:03
@jsoriano
Copy link
Copy Markdown
Member Author

Failing tests caused by #12171

@jsoriano jsoriano force-pushed the test-close-on-signal branch from 575e14c to 02eeabb Compare May 20, 2019 10:03
@jsoriano
Copy link
Copy Markdown
Member Author

@exekias @andrewkroh this would be ready for review, thanks!

@jsoriano
Copy link
Copy Markdown
Member Author

Test failure in Windows looks related, I'll look at this

time.Sleep(10 * time.Millisecond)
}
profile := pprof.Lookup("goroutine")
profile.WriteTo(os.Stdout, 2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice. That's helpful.

@jsoriano jsoriano merged commit f841521 into elastic:master May 24, 2019
@jsoriano jsoriano deleted the test-close-on-signal branch May 24, 2019 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants