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

Backport state pool leak fixes to 2.0 branch #6713

Merged
merged 4 commits into from Dec 15, 2016

Conversation

Projects
None yet
2 participants
@babbageclunk
Copy link
Member

commented Dec 15, 2016

Fix state pool leaks (gets without corresponding releases) from code using the httpContext methods to get a state object. Backports of #6566 and #6703 to the 2.0 branch.

Fixes https://bugs.launchpad.net/juju/+bug/1641824 - although migrations aren't available in 2.0, the state pool would also leak when models are destroyed.

babbageclunk added some commits Nov 15, 2016

Fix StatePool leaks
Part of http://pad.lv/1641824 - the migration back doesn't work because
the state references in the pool never get to 0, so they're marked for
removal but not actually removed.

Ensure that state references returned from
httpContext.stateForRequestUnauthenticated get released back to the
StatePool when they're finished with.

In the logstream handler. since the state is created in the newSource
closure and returned, the normal defer method doesn't work. Change
newSource to also return a closer function that will release the state
back to the pool and call that closer func from
logStreamRequestHandler.close().
Ensure resource HTTP handlers release state
Thread the call to httpContext.release through so it can be called from
the right place in the resource HTTP handler.
Close state leaks from the NewResourceOpener code
This code path is a bit trickier to close than the other handler.
@babbageclunk

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2016

!!build!!

@babbageclunk

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2016

$$merge$$

@jujubot

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9884

@babbageclunk

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2016

Spurious lxd failure.

$$merge$$

@jujubot

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 694df72 into juju:2.0 Dec 15, 2016

1 check failed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details

@babbageclunk babbageclunk deleted the babbageclunk:resource-state-leak-2.0 branch Dec 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.