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.

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

don't wait to expire dead endpoints if exiting #964

Merged
merged 1 commit into from
Apr 3, 2020
Merged

don't wait to expire dead endpoints if exiting #964

merged 1 commit into from
Apr 3, 2020

Conversation

MHBauer
Copy link
Contributor

@MHBauer MHBauer commented Mar 31, 2020

Type of change

  • Bug fix

Description

deadEndpoints channel gets filled up. Exit occurs, and deadEndpoints is not drained. We can avoid putting more into the channel and creating hanging goroutines (many many many of them, this fills up my memory). There are still messages that get dropped on the floor sometimes that will cause a different hang.

Additional details

Seeing what looked like unbounded memory usage when running this in a loop.
There are anywhere between 80 and 600 messages for each side of the connection.
Each iteration through the loop results in a timer being created.
Avoid that and create one at the test start.

Related issues

FAB-17451 I reported this from #954 (comment)

@MHBauer MHBauer requested a review from a team as a code owner March 31, 2020 19:42
@MHBauer
Copy link
Contributor Author

MHBauer commented Mar 31, 2020

@denyeart maybe #3 from your bug. The build log had expired.

return
select {
case c.deadEndpoints <- pkiID:
case <-c.exitChan:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe return here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

@ryjones ryjones requested a review from a team March 31, 2020 22:37
@MHBauer MHBauer requested a review from yacovm March 31, 2020 22:53
@MHBauer
Copy link
Contributor Author

MHBauer commented Apr 1, 2020

That would be the other flake I mean.

/ci-run

@github-actions
Copy link

github-actions bot commented Apr 1, 2020

AZP build triggered!

@MHBauer
Copy link
Contributor Author

MHBauer commented Apr 1, 2020

flake is https://jira.hyperledger.org/browse/FAB-16233

/ci-run

@github-actions
Copy link

github-actions bot commented Apr 1, 2020

AZP build triggered!

Copy link
Contributor

@yacovm yacovm left a comment

Choose a reason for hiding this comment

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

Please keep this pull request focused and not have in it commits that are unrelated

Change-Id: I4253ab37d34ae90d60532f7769cccd751c799fdd
Signed-off-by: Morgan Bauer <mbauer@us.ibm.com>
@MHBauer
Copy link
Contributor Author

MHBauer commented Apr 3, 2020

moved to separate PR #983. I would suggest merging that one first if this keeps flakin.

@MHBauer MHBauer requested a review from yacovm April 3, 2020 01:44
@yacovm yacovm merged commit 6ecdaa3 into hyperledger:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants