Skip to content

Commit 2f173b8

Browse files
axwaxw
committed
state: introduce StatePoolReleaser
Change StatePool.Get to return a StatePoolReleaser, rather than func(), and have it report whether or not releasing caused the State object to be removed from the pool. Also update the Remove method to report the same (i.e. that removing actually caused the removal immediately due to 0 references.)
1 parent 5587a28 commit 2f173b8

20 files changed

+110
-74
lines changed

‎apiserver/apiserver.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ func (srv *Server) endpoints() []apihttp.Endpoint {
596596
)
597597

598598
add("/model/:modeluuid/applications/:application/resources/:resource", &ResourcesHandler{
599-
StateAuthFunc: func(req *http.Request, tagKinds ...string) (ResourcesBackend, func(), names.Tag, error) {
599+
StateAuthFunc: func(req *http.Request, tagKinds ...string) (ResourcesBackend, state.StatePoolReleaser, names.Tag, error) {
600600
st, closer, entity, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
601601
if err != nil {
602602
return nil, nil, nil, errors.Trace(err)
@@ -609,7 +609,7 @@ func (srv *Server) endpoints() []apihttp.Endpoint {
609609
},
610610
})
611611
add("/model/:modeluuid/units/:unit/resources/:resource", &UnitResourcesHandler{
612-
NewOpener: func(req *http.Request, tagKinds ...string) (resource.Opener, func(), error) {
612+
NewOpener: func(req *http.Request, tagKinds ...string) (resource.Opener, state.StatePoolReleaser, error) {
613613
st, closer, _, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
614614
if err != nil {
615615
return nil, nil, errors.Trace(err)
@@ -827,7 +827,7 @@ func (srv *Server) serveConn(wsConn *websocket.Conn, modelUUID string, apiObserv
827827
var (
828828
st *state.State
829829
h *apiHandler
830-
releaser func()
830+
releaser state.StatePoolReleaser
831831
)
832832
if err == nil {
833833
st, releaser, err = srv.statePool.Get(resolvedModelUUID)
@@ -976,8 +976,7 @@ func (srv *Server) processModelRemovals() error {
976976
// Model's gone away - ensure that it gets removed
977977
// from from the state pool once people are finished
978978
// with it.
979-
err = srv.statePool.Remove(modelUUID)
980-
if err != nil {
979+
if _, err := srv.statePool.Remove(modelUUID); err != nil {
981980
return errors.Trace(err)
982981
}
983982
}

‎apiserver/applicationoffers/state.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,13 @@ type statePoolShim struct {
2929
}
3030

3131
func (pool statePoolShim) Get(modelUUID string) (Backend, func(), error) {
32-
st, closer, err := pool.StatePool.Get(modelUUID)
32+
st, releaser, err := pool.StatePool.Get(modelUUID)
3333
if err != nil {
3434
return nil, nil, errors.Trace(err)
3535
}
36+
closer := func() {
37+
releaser()
38+
}
3639
return &stateShim{st}, closer, nil
3740
}
3841

‎apiserver/charms.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func (h *CharmsHTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7676
type charmsHandler struct {
7777
ctxt httpContext
7878
dataDir string
79-
stateAuthFunc func(*http.Request) (*state.State, func(), error)
79+
stateAuthFunc func(*http.Request) (*state.State, state.StatePoolReleaser, error)
8080
}
8181

8282
// bundleContentSenderFunc functions are responsible for sending a

‎apiserver/httpcontext.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type httpContext struct {
3232
// stateForRequestUnauthenticated returns a state instance appropriate for
3333
// using for the model implicit in the given request
3434
// without checking any authentication information.
35-
func (ctxt *httpContext) stateForRequestUnauthenticated(r *http.Request) (*state.State, func(), error) {
35+
func (ctxt *httpContext) stateForRequestUnauthenticated(r *http.Request) (*state.State, state.StatePoolReleaser, error) {
3636
modelUUID, err := validateModelUUID(validateArgs{
3737
statePool: ctxt.srv.statePool,
3838
modelUUID: r.URL.Query().Get(":modeluuid"),
@@ -53,7 +53,11 @@ func (ctxt *httpContext) stateForRequestUnauthenticated(r *http.Request) (*state
5353
// using for the model implicit in the given request.
5454
// It also returns the authenticated entity.
5555
func (ctxt *httpContext) stateForRequestAuthenticated(r *http.Request) (
56-
resultSt *state.State, resultReleaser func(), resultEntity state.Entity, err error) {
56+
resultSt *state.State,
57+
resultReleaser state.StatePoolReleaser,
58+
resultEntity state.Entity,
59+
err error,
60+
) {
5761
st, releaser, err := ctxt.stateForRequestUnauthenticated(r)
5862
if err != nil {
5963
return nil, nil, nil, errors.Trace(err)
@@ -116,7 +120,10 @@ func checkPermissions(tag names.Tag, acceptFunc common.GetAuthFunc) (bool, error
116120
// has admin permissions on the controller model. The method also gets the
117121
// model uuid for the model being migrated from a request header, and returns
118122
// the state instance for that model.
119-
func (ctxt *httpContext) stateForMigration(r *http.Request, requiredMode state.MigrationMode) (st *state.State, returnReleaser func(), err error) {
123+
func (ctxt *httpContext) stateForMigration(
124+
r *http.Request,
125+
requiredMode state.MigrationMode,
126+
) (st *state.State, returnReleaser state.StatePoolReleaser, err error) {
120127
var user state.Entity
121128
st, releaser, user, err := ctxt.stateAndEntityForRequestAuthenticatedUser(r)
122129
if err != nil {
@@ -165,33 +172,33 @@ func (ctxt *httpContext) stateForMigration(r *http.Request, requiredMode state.M
165172
return migrationSt, migrationReleaser, nil
166173
}
167174

168-
func (ctxt *httpContext) stateForMigrationImporting(r *http.Request) (*state.State, func(), error) {
175+
func (ctxt *httpContext) stateForMigrationImporting(r *http.Request) (*state.State, state.StatePoolReleaser, error) {
169176
return ctxt.stateForMigration(r, state.MigrationModeImporting)
170177
}
171178

172179
// stateForRequestAuthenticatedUser is like stateAndEntityForRequestAuthenticatedUser
173180
// but doesn't return the entity.
174-
func (ctxt *httpContext) stateForRequestAuthenticatedUser(r *http.Request) (*state.State, func(), error) {
181+
func (ctxt *httpContext) stateForRequestAuthenticatedUser(r *http.Request) (*state.State, state.StatePoolReleaser, error) {
175182
st, releaser, _, err := ctxt.stateAndEntityForRequestAuthenticatedUser(r)
176183
return st, releaser, err
177184
}
178185

179186
// stateAndEntityForRequestAuthenticatedUser is like stateForRequestAuthenticated
180187
// except that it also verifies that the authenticated entity is a user.
181-
func (ctxt *httpContext) stateAndEntityForRequestAuthenticatedUser(r *http.Request) (*state.State, func(), state.Entity, error) {
188+
func (ctxt *httpContext) stateAndEntityForRequestAuthenticatedUser(r *http.Request) (*state.State, state.StatePoolReleaser, state.Entity, error) {
182189
return ctxt.stateForRequestAuthenticatedTag(r, names.UserTagKind)
183190
}
184191

185192
// stateForRequestAuthenticatedAgent is like stateForRequestAuthenticated
186193
// except that it also verifies that the authenticated entity is an agent.
187-
func (ctxt *httpContext) stateForRequestAuthenticatedAgent(r *http.Request) (*state.State, func(), state.Entity, error) {
194+
func (ctxt *httpContext) stateForRequestAuthenticatedAgent(r *http.Request) (*state.State, state.StatePoolReleaser, state.Entity, error) {
188195
return ctxt.stateForRequestAuthenticatedTag(r, names.MachineTagKind, names.UnitTagKind)
189196
}
190197

191198
// stateForRequestAuthenticatedTag checks that the request is
192199
// correctly authenticated, and that the authenticated entity making
193200
// the request is of one of the specified kinds.
194-
func (ctxt *httpContext) stateForRequestAuthenticatedTag(r *http.Request, kinds ...string) (*state.State, func(), state.Entity, error) {
201+
func (ctxt *httpContext) stateForRequestAuthenticatedTag(r *http.Request, kinds ...string) (*state.State, state.StatePoolReleaser, state.Entity, error) {
195202
funcs := make([]common.GetAuthFunc, len(kinds))
196203
for i, kind := range kinds {
197204
funcs[i] = common.AuthFuncForTagKind(kind)

‎apiserver/logsink.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type agentLoggingStrategy struct {
2323
fileLogger io.Writer
2424

2525
st *state.State
26-
releaser func()
26+
releaser state.StatePoolReleaser
2727
version version.Number
2828
entity names.Tag
2929
filePrefix string

‎apiserver/logstream.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,14 @@ type messageWriter interface {
2727
WriteJSON(v interface{}) error
2828
}
2929

30-
type closerFunc func()
31-
3230
// logStreamEndpointHandler takes requests to stream logs from the DB.
3331
type logStreamEndpointHandler struct {
3432
stopCh <-chan struct{}
35-
newSource func(*http.Request) (logStreamSource, closerFunc, error)
33+
newSource func(*http.Request) (logStreamSource, state.StatePoolReleaser, error)
3634
}
3735

3836
func newLogStreamEndpointHandler(ctxt httpContext) *logStreamEndpointHandler {
39-
newSource := func(req *http.Request) (logStreamSource, closerFunc, error) {
37+
newSource := func(req *http.Request) (logStreamSource, state.StatePoolReleaser, error) {
4038
st, releaser, _, err := ctxt.stateForRequestAuthenticatedAgent(req)
4139
if err != nil {
4240
return nil, nil, errors.Trace(err)
@@ -78,13 +76,13 @@ func (h *logStreamEndpointHandler) newLogStreamRequestHandler(conn messageWriter
7876
// Validate before authenticate because the authentication is
7977
// dependent on the state connection that is determined during the
8078
// validation.
81-
source, closer, err := h.newSource(req)
79+
source, releaser, err := h.newSource(req)
8280
if err != nil {
8381
return nil, errors.Trace(err)
8482
}
8583
defer func() {
8684
if err != nil {
87-
closer()
85+
releaser()
8886
}
8987
}()
9088

@@ -101,10 +99,10 @@ func (h *logStreamEndpointHandler) newLogStreamRequestHandler(conn messageWriter
10199
}
102100

103101
reqHandler := &logStreamRequestHandler{
104-
conn: conn,
105-
req: req,
106-
tailer: tailer,
107-
closer: closer,
102+
conn: conn,
103+
req: req,
104+
tailer: tailer,
105+
releaser: releaser,
108106
}
109107
return reqHandler, nil
110108
}
@@ -184,10 +182,10 @@ func (st logStreamState) newTailer(args state.LogTailerParams) (state.LogTailer,
184182
}
185183

186184
type logStreamRequestHandler struct {
187-
conn messageWriter
188-
req *http.Request
189-
tailer state.LogTailer
190-
closer closerFunc
185+
conn messageWriter
186+
req *http.Request
187+
tailer state.LogTailer
188+
releaser state.StatePoolReleaser
191189
}
192190

193191
func (h *logStreamRequestHandler) serveWebsocket(stop <-chan struct{}) {
@@ -217,7 +215,7 @@ func (h *logStreamRequestHandler) serveWebsocket(stop <-chan struct{}) {
217215

218216
func (h *logStreamRequestHandler) close() {
219217
h.tailer.Stop()
220-
h.closer()
218+
h.releaser()
221219
}
222220

223221
func (h *logStreamRequestHandler) sendRecords(rec []*state.LogRecord) error {

‎apiserver/logstream_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,15 @@ type stubSource struct {
254254
ReturnNewTailer state.LogTailer
255255
}
256256

257-
func (s *stubSource) newSource(req *http.Request) (logStreamSource, closerFunc, error) {
257+
func (s *stubSource) newSource(req *http.Request) (logStreamSource, state.StatePoolReleaser, error) {
258258
s.stub.AddCall("newSource", req)
259259
if err := s.stub.NextErr(); err != nil {
260260
return nil, nil, errors.Trace(err)
261261
}
262262

263-
closer := func() {
263+
closer := func() bool {
264264
s.stub.AddCall("close")
265+
return false
265266
}
266267
return s, closer, nil
267268
}

‎apiserver/logtransfer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818

1919
type migrationLoggingStrategy struct {
2020
st *state.State
21-
releaser func()
21+
releaser state.StatePoolReleaser
2222
filePrefix string
2323
dbLogger *state.DbLogger
2424
tracker *logTracker

‎apiserver/modelmanager/modelmanager.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ type statePool struct {
114114
// Get implements StatePool.
115115
func (p *statePool) Get(modelUUID string) (common.ModelManagerBackend, func(), error) {
116116
st, releaser, err := p.pool.Get(modelUUID)
117-
return common.NewModelManagerBackend(st), releaser, err
117+
closer := func() {
118+
releaser()
119+
}
120+
return common.NewModelManagerBackend(st), closer, err
118121
}
119122

120123
// NewModelManagerAPI creates a new api server endpoint for managing

‎apiserver/remoterelations/state.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,13 @@ type statePoolShim struct {
194194
}
195195

196196
func (pool statePoolShim) Get(modelUUID string) (RemoteRelationsState, func(), error) {
197-
st, closer, err := pool.StatePool.Get(modelUUID)
197+
st, releaser, err := pool.StatePool.Get(modelUUID)
198198
if err != nil {
199199
return nil, nil, errors.Trace(err)
200200
}
201+
closer := func() {
202+
releaser()
203+
}
201204
return stateShim{st}, closer, nil
202205
}
203206

‎apiserver/resources.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/juju/juju/apiserver/params"
1919
"github.com/juju/juju/resource"
2020
"github.com/juju/juju/resource/api"
21+
"github.com/juju/juju/state"
2122
)
2223

2324
// ResourcesBackend is the functionality of Juju's state needed for the resources API.
@@ -41,7 +42,7 @@ type ResourcesBackend interface {
4142
// ResourcesHandler is the HTTP handler for client downloads and
4243
// uploads of resources.
4344
type ResourcesHandler struct {
44-
StateAuthFunc func(*http.Request, ...string) (ResourcesBackend, func(), names.Tag, error)
45+
StateAuthFunc func(*http.Request, ...string) (ResourcesBackend, state.StatePoolReleaser, names.Tag, error)
4546
}
4647

4748
// ServeHTTP implements http.Handler.

‎apiserver/resources_mig.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
// resourcesMigrationUploadHandler handles resources uploads for model migrations.
2121
type resourcesMigrationUploadHandler struct {
2222
ctxt httpContext
23-
stateAuthFunc func(*http.Request) (*state.State, func(), error)
23+
stateAuthFunc func(*http.Request) (*state.State, state.StatePoolReleaser, error)
2424
}
2525

2626
func (h *resourcesMigrationUploadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

‎apiserver/resources_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/juju/juju/resource"
2727
"github.com/juju/juju/resource/api"
2828
"github.com/juju/juju/resource/resourcetesting"
29+
"github.com/juju/juju/state"
2930
)
3031

3132
type ResourcesHandlerSuite struct {
@@ -61,11 +62,11 @@ func (s *ResourcesHandlerSuite) SetUpTest(c *gc.C) {
6162
}
6263
}
6364

64-
func (s *ResourcesHandlerSuite) authState(req *http.Request, tagKinds ...string) (apiserver.ResourcesBackend, func(), names.Tag, error) {
65+
func (s *ResourcesHandlerSuite) authState(req *http.Request, tagKinds ...string) (apiserver.ResourcesBackend, state.StatePoolReleaser, names.Tag, error) {
6566
if s.stateAuthErr != nil {
6667
return nil, nil, nil, errors.Trace(s.stateAuthErr)
6768
}
68-
closer := func() {}
69+
closer := func() bool { return false }
6970
tag := names.NewUserTag(s.username)
7071
return s.backend, closer, tag, nil
7172
}

‎apiserver/resources_unit.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ import (
1414
"github.com/juju/juju/apiserver/params"
1515
"github.com/juju/juju/resource"
1616
"github.com/juju/juju/resource/api"
17+
"github.com/juju/juju/state"
1718
)
1819

1920
// ResourcesHandler is the HTTP handler for unit agent downloads of
2021
// resources.
2122
type UnitResourcesHandler struct {
22-
NewOpener func(*http.Request, ...string) (resource.Opener, func(), error)
23+
NewOpener func(*http.Request, ...string) (resource.Opener, state.StatePoolReleaser, error)
2324
}
2425

2526
// ServeHTTP implements http.Handler.

‎apiserver/resources_unit_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/juju/juju/apiserver"
1717
"github.com/juju/juju/resource"
1818
"github.com/juju/juju/resource/resourcetesting"
19+
"github.com/juju/juju/state"
1920
)
2021

2122
type UnitResourcesHandlerSuite struct {
@@ -41,8 +42,9 @@ func (s *UnitResourcesHandlerSuite) SetUpTest(c *gc.C) {
4142
s.recorder = httptest.NewRecorder()
4243
}
4344

44-
func (s *UnitResourcesHandlerSuite) closer() {
45+
func (s *UnitResourcesHandlerSuite) closer() bool {
4546
s.stub.AddCall("Close")
47+
return false
4648
}
4749

4850
func (s *UnitResourcesHandlerSuite) TestWrongMethod(c *gc.C) {
@@ -60,7 +62,7 @@ func (s *UnitResourcesHandlerSuite) TestWrongMethod(c *gc.C) {
6062
func (s *UnitResourcesHandlerSuite) TestOpenerCreationError(c *gc.C) {
6163
failure, expectedBody := apiFailure("boom", "")
6264
handler := &apiserver.UnitResourcesHandler{
63-
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, func(), error) {
65+
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, state.StatePoolReleaser, error) {
6466
return nil, nil, failure
6567
},
6668
}
@@ -84,7 +86,7 @@ func (s *UnitResourcesHandlerSuite) TestOpenResourceError(c *gc.C) {
8486
failure, expectedBody := apiFailure("boom", "")
8587
s.stub.SetErrors(failure)
8688
handler := &apiserver.UnitResourcesHandler{
87-
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, func(), error) {
89+
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, state.StatePoolReleaser, error) {
8890
s.stub.AddCall("NewOpener", kinds)
8991
return opener, s.closer, nil
9092
},
@@ -111,7 +113,7 @@ func (s *UnitResourcesHandlerSuite) TestSuccess(c *gc.C) {
111113
ReturnOpenResource: opened,
112114
}
113115
handler := &apiserver.UnitResourcesHandler{
114-
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, func(), error) {
116+
NewOpener: func(_ *http.Request, kinds ...string) (resource.Opener, state.StatePoolReleaser, error) {
115117
s.stub.AddCall("NewOpener", kinds)
116118
return opener, s.closer, nil
117119
},

‎apiserver/rest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (h *RestHTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
4444
type modelRestHandler struct {
4545
ctxt httpContext
4646
dataDir string
47-
stateAuthFunc func(*http.Request) (*state.State, func(), error)
47+
stateAuthFunc func(*http.Request) (*state.State, state.StatePoolReleaser, error)
4848
}
4949

5050
// ServeGet handles http GET requests.

‎apiserver/tools.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
// toolsHandler handles tool upload through HTTPS in the API server.
3131
type toolsUploadHandler struct {
3232
ctxt httpContext
33-
stateAuthFunc func(*http.Request) (*state.State, func(), error)
33+
stateAuthFunc func(*http.Request) (*state.State, state.StatePoolReleaser, error)
3434
}
3535

3636
// toolsHandler handles tool download through HTTPS in the API server.

0 commit comments

Comments
 (0)