Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Track the locations of reference sources for the state pool. #6879
Conversation
babbageclunk
approved these changes
Jan 27, 2017
I like it! This should be a great help in chasing down state leaks.
| - // closed. | ||
| - Release func(*state.State) error | ||
| + // for the given HTTP request. It is the caller's responsibility | ||
| + // to call the release function returned. If the error arg is non-nil |
babbageclunk
Jan 27, 2017
Member
Shouldn't this be "If the error return value is nil the release function will always be a valid function."?
| - references uint | ||
| - remove bool | ||
| + state *State | ||
| + references uint |
babbageclunk
Jan 27, 2017
Member
If you've got referenceSources you don't really need references - I'd just get rid of this and express everything in terms of referenceSources.
| @@ -35,14 +39,17 @@ type StatePool struct { | ||
| // mu protects pool | ||
| mu sync.Mutex | ||
| pool map[string]*PoolItem | ||
| + // soureKey is used to provide a unique number as a key for the | ||
| + // referencesSources structure in the pool. | ||
| + soureKey uint64 |
| + } | ||
| + released = true | ||
| + } | ||
| + source := string(debug.Stack()) |
babbageclunk
Jan 27, 2017
Member
Do we know how expensive calling debug.Stack() is? I mean, it's the only way we can get this information so I guess we have to do it, but given that the whole point of the statepool is to speed up using state, it would be sad if getting the stack is very costly.
howbazaar
Feb 7, 2017
Owner
it is pretty quick, and certainly much faster than any access of mongo we'll be doing
| @@ -29,10 +29,17 @@ type DepEngineReporter interface { | ||
| Report() map[string]interface{} | ||
| } | ||
| +// IntrospectionReporter provider a simple method that the introspection |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
howbazaar commentedJan 27, 2017
In the continuing saga of tracking memory leaks this branch adds introspection into the state pool that is used by the apiserver.
The state pool now records the stack whenever the Get method is called, and clears that stack when the releaser is called.
QA steps