Use pprof profile for tracking state instances. #6959

Merged
merged 1 commit into from Feb 10, 2017

Conversation

Projects
None yet
3 participants
Owner

howbazaar commented Feb 9, 2017

Unfortunately using the runtime finalizers didn't work as expected as if there is a cycle in the structures, as there is in the state object and the workers, the finalizers are not run even thought the objects are garbage collected.

The tracker now removes the references when the state instances are closed. Now that we were doing that, it made no sense to write our own tracking code when pprof does that perfectly well.

QA steps

juju bootstrap
juju ssh 0
juju-statetracker-report

Owner

howbazaar commented Feb 9, 2017

Report now looks like this:

HTTP/1.0 200 OK
Content-Type: text/plain; charset=utf-8
Date: Thu, 09 Feb 2017 23:07:34 GMT

juju/state/tracker profile: total 4
1 @ 0x720641 0x71aae5 0x71a250 0x588649 0x584330 0x58d5a2 0x1644a18 0xb584c2 0xb5866f 0xb55886 0x478141
#	0x720641	github.com/juju/juju/state.newState+0x4d1									/home/tim/go/src/github.com/juju/juju/state/open.go:557
#	0x71aae5	github.com/juju/juju/state.open+0x405										/home/tim/go/src/github.com/juju/juju/state/open.go:136
#	0x71a250	github.com/juju/juju/state.Open+0x140										/home/tim/go/src/github.com/juju/juju/state/open.go:95
#	0x588649	github.com/juju/juju/cmd/jujud/agent.openState+0x2e9								/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:1451
#	0x584330	github.com/juju/juju/cmd/jujud/agent.(*MachineAgent).initState+0x130						/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:947
#	0x58d5a2	github.com/juju/juju/cmd/jujud/agent.(*MachineAgent).(github.com/juju/juju/cmd/jujud/agent.initState)-fm+0x42	/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:530
#	0x1644a18	github.com/juju/juju/worker/state.Manifold.func1+0x2b8								/home/tim/go/src/github.com/juju/juju/worker/state/manifold.go:62
#	0xb584c2	github.com/juju/juju/worker/dependency.(*Engine).runWorker.func1+0x522						/home/tim/go/src/github.com/juju/juju/worker/dependency/engine.go:432
#	0xb5866f	github.com/juju/juju/worker/dependency.(*Engine).runWorker.func2+0x8f						/home/tim/go/src/github.com/juju/juju/worker/dependency/engine.go:436
#	0xb55886	github.com/juju/juju/worker/dependency.(*Engine).runWorker+0x1e6						/home/tim/go/src/github.com/juju/juju/worker/dependency/engine.go:468

1 @ 0x720641 0x71aae5 0x71a250 0x588649 0x58f606 0x58fe6e 0xb4fff1 0x478141
#	0x720641	github.com/juju/juju/state.newState+0x4d1						/home/tim/go/src/github.com/juju/juju/state/open.go:557
#	0x71aae5	github.com/juju/juju/state.open+0x405							/home/tim/go/src/github.com/juju/juju/state/open.go:136
#	0x71a250	github.com/juju/juju/state.Open+0x140							/home/tim/go/src/github.com/juju/juju/state/open.go:95
#	0x588649	github.com/juju/juju/cmd/jujud/agent.openState+0x2e9					/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:1451
#	0x58f606	github.com/juju/juju/cmd/jujud/agent.(*MachineAgent).startStateWorkers.func6+0x106	/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:1047
#	0x58fe6e	github.com/juju/juju/cmd/jujud/agent.(*MachineAgent).apiserverWorkerStarter.func1+0x6e	/home/tim/go/src/github.com/juju/juju/cmd/jujud/agent/machine.go:1143
#	0xb4fff1	github.com/juju/juju/worker.(*runner).runWorker+0x431					/home/tim/go/src/github.com/juju/juju/worker/runner.go:257

2 @ 0x720641 0x761df5 0x7284b0 0xc07c85 0xc3b2ea 0x1199d72 0x1199a8e 0xc078b0 0xc3a72e 0xbab4ca 0xc3b045 0xbab4ca 0x15a08d7 0xbad94e 0xbaa22e 0x478141
#	0x720641	github.com/juju/juju/state.newState+0x4d1							/home/tim/go/src/github.com/juju/juju/state/open.go:557
#	0x761df5	github.com/juju/juju/state.(*State).ForModel+0xc5						/home/tim/go/src/github.com/juju/juju/state/state.go:315
#	0x7284b0	github.com/juju/juju/state.(*StatePool).Get+0x590						/home/tim/go/src/github.com/juju/juju/state/pool.go:91
#	0xc07c85	github.com/juju/juju/apiserver.(*Server).serveConn+0x335					/home/tim/go/src/github.com/juju/juju/apiserver/apiserver.go:668
#	0xc3b2ea	github.com/juju/juju/apiserver.(*Server).apiHandler.func2+0x21a					/home/tim/go/src/github.com/juju/juju/apiserver/apiserver.go:642
#	0x1199d72	golang.org/x/net/websocket.Server.serveWebSocket+0x2d2						/home/tim/go/src/golang.org/x/net/websocket/server.go:89
#	0x1199a8e	golang.org/x/net/websocket.Server.ServeHTTP+0x4e						/home/tim/go/src/golang.org/x/net/websocket/server.go:70
#	0xc078b0	github.com/juju/juju/apiserver.(*Server).apiHandler+0x230					/home/tim/go/src/github.com/juju/juju/apiserver/apiserver.go:647
#	0xc3a72e	github.com/juju/juju/apiserver.(*Server).(github.com/juju/juju/apiserver.apiHandler)-fm+0x3e	/home/tim/go/src/github.com/juju/juju/apiserver/apiserver.go:413
#	0xbab4ca	net/http.HandlerFunc.ServeHTTP+0x3a								/usr/lib/go-1.6/src/net/http/server.go:1618
#	0xc3b045	github.com/juju/juju/apiserver.(*Server).trackRequests.func1+0x145				/home/tim/go/src/github.com/juju/juju/apiserver/apiserver.go:612
#	0xbab4ca	net/http.HandlerFunc.ServeHTTP+0x3a								/usr/lib/go-1.6/src/net/http/server.go:1618
#	0x15a08d7	github.com/bmizerany/pat.(*PatternServeMux).ServeHTTP+0x257					/home/tim/go/src/github.com/bmizerany/pat/mux.go:117
#	0xbad94e	net/http.serverHandler.ServeHTTP+0x19e								/usr/lib/go-1.6/src/net/http/server.go:2081
#	0xbaa22e	net/http.(*conn).serve+0xf2e									/usr/lib/go-1.6/src/net/http/server.go:1472
@@ -549,7 +553,7 @@ func newState(
st.policy = newPolicy(st)
}
// Record this State instance with the global tracker.
- GlobalTracker.Add(st)
+ profileTracker.Add(st, 1)
@babbageclunk

babbageclunk Feb 9, 2017

Member

I guess we aren't worried about the fact that this keeps a reference to the state? If the state isn't closed the worker goroutines will keep it alive anyway.

@@ -1,157 +0,0 @@
-// Copyright 2017 Canonical Ltd.
@babbageclunk

babbageclunk Feb 9, 2017

Member

Yay deletia.

@@ -82,7 +82,7 @@ juju-statepool-report () {
}
juju-statetracker-report () {
- jujuMachineOrUnit statetracker/ $@
+ jujuMachineOrUnit debug/pprof/juju/state/tracker?debug=1 $@
Member

babbageclunk commented Feb 9, 2017

Do we want to hide this behind the developer flag? Would be good to do a check to see whether there's any perf impact.

Owner

howbazaar commented Feb 9, 2017

I don't want it behind a developer flag because I want to be able to get access to it from other production environments.

$$merge$$

Contributor

jujubot commented Feb 9, 2017

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

@jujubot jujubot merged commit c22c012 into juju:2.1 Feb 10, 2017

@howbazaar howbazaar deleted the howbazaar:2.1-state-tracker branch Feb 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment