Merge pull request juju#8392 from manadart/pooled-state-over-callbacks

ObviouslyGreen · Feb 19, 2018 · 742a810 · 742a810
2 parents b50ddc4 + 8e1a81e
commit 742a810
Show file tree

Hide file tree

Showing 61 changed files with 627 additions and 517 deletions.
diff --git a/agent/agentbootstrap/bootstrap_test.go b/agent/agentbootstrap/bootstrap_test.go
@@ -257,9 +257,9 @@ LXC_BRIDGE="ignored"`[1:])
 	// controller model.
 	statePool := state.NewStatePool(st)
 	defer statePool.Close()
-	hostedModelSt, release, err := statePool.Get(hostedModelUUID)
+	hostedModelSt, err := statePool.Get(hostedModelUUID)
 	c.Assert(err, jc.ErrorIsNil)
-	defer release()
+	defer hostedModelSt.Release()
 	gotModelConstraints, err = hostedModelSt.ModelConstraints()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(gotModelConstraints, gc.DeepEquals, expectModelConstraints)

diff --git a/api/watcher/watcher_test.go b/api/watcher/watcher_test.go
@@ -11,7 +11,7 @@ import (
 	"github.com/juju/utils"
 	gc "gopkg.in/check.v1"
 	"gopkg.in/juju/names.v2"
-	worker "gopkg.in/juju/worker.v1"
+	"gopkg.in/juju/worker.v1"
 	"gopkg.in/macaroon-bakery.v1/bakery"
 	"gopkg.in/macaroon-bakery.v1/bakery/checkers"
 	"gopkg.in/macaroon.v1"
@@ -511,10 +511,10 @@ type migrationSuite struct {
 var _ = gc.Suite(&migrationSuite{})
 
 func (s *migrationSuite) startSync(c *gc.C, st *state.State) {
-	backingSt, releaser, err := s.BackingStatePool.Get(st.ModelUUID())
+	backingSt, err := s.BackingStatePool.Get(st.ModelUUID())
 	c.Assert(err, jc.ErrorIsNil)
 	backingSt.StartSync()
-	releaser()
+	backingSt.Release()
 }
 
 func (s *migrationSuite) TestMigrationStatusWatcher(c *gc.C) {

diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go
@@ -931,9 +931,9 @@ func (s *loginSuite) assertRemoteModel(c *gc.C, api api.Connection, expected nam
 	// the expected model. We make a change in state on that model, and
 	// then check that it is picked up by a call to the API.
 
-	st, release, err := s.StatePool.Get(tag.Id())
+	st, err := s.StatePool.Get(tag.Id())
 	c.Assert(err, jc.ErrorIsNil)
-	defer release()
+	defer st.Release()
 
 	expectedCons := constraints.MustParse("mem=8G")
 	err = st.SetModelConstraints(expectedCons)

diff --git a/apiserver/apiserver.go b/apiserver/apiserver.go
@@ -585,21 +585,21 @@ func (srv *Server) endpoints() []apihttp.Endpoint {
 	)
 
 	add("/model/:modeluuid/applications/:application/resources/:resource", &ResourcesHandler{
-		StateAuthFunc: func(req *http.Request, tagKinds ...string) (ResourcesBackend, state.StatePoolReleaser, names.Tag, error) {
-			st, closer, entity, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
+		StateAuthFunc: func(req *http.Request, tagKinds ...string) (ResourcesBackend, state.PoolHelper, names.Tag, error) {
+			st, entity, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
 			if err != nil {
 				return nil, nil, nil, errors.Trace(err)
 			}
 			rst, err := st.Resources()
 			if err != nil {
 				return nil, nil, nil, errors.Trace(err)
 			}
-			return rst, closer, entity.Tag(), nil
+			return rst, st, entity.Tag(), nil
 		},
 	})
 	add("/model/:modeluuid/units/:unit/resources/:resource", &UnitResourcesHandler{
-		NewOpener: func(req *http.Request, tagKinds ...string) (resource.Opener, state.StatePoolReleaser, error) {
-			st, closer, _, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
+		NewOpener: func(req *http.Request, tagKinds ...string) (resource.Opener, state.PoolHelper, error) {
+			st, _, err := httpCtxt.stateForRequestAuthenticatedTag(req, tagKinds...)
 			if err != nil {
 				return nil, nil, errors.Trace(err)
 			}
@@ -608,11 +608,11 @@ func (srv *Server) endpoints() []apihttp.Endpoint {
 			if err != nil {
 				return nil, nil, errors.Trace(err)
 			}
-			opener, err := resourceadapters.NewResourceOpener(st, tag.Id())
+			opener, err := resourceadapters.NewResourceOpener(st.State, tag.Id())
 			if err != nil {
 				return nil, nil, errors.Trace(err)
 			}
-			return opener, closer, nil
+			return opener, st, nil
 		},
 	})
 
@@ -831,17 +831,16 @@ func (srv *Server) serveConn(
 		modelUUID: modelUUID,
 	})
 	var (
-		st       *state.State
-		h        *apiHandler
-		releaser state.StatePoolReleaser
+		st *state.PooledState
+		h  *apiHandler
 	)
 	if err == nil {
-		st, releaser, err = srv.statePool.Get(resolvedModelUUID)
+		st, err = srv.statePool.Get(resolvedModelUUID)
 	}
 
 	if err == nil {
-		defer releaser()
-		h, err = newAPIHandler(srv, st, conn, modelUUID, connectionID, host)
+		defer st.Release()
+		h, err = newAPIHandler(srv, st.State, conn, modelUUID, connectionID, host)
 	}
 
 	if err != nil {

diff --git a/apiserver/backup.go b/apiserver/backup.go
@@ -36,20 +36,20 @@ type backupHandler struct {
 func (h *backupHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 	// Validate before authenticate because the authentication is dependent
 	// on the state connection that is determined during the validation.
-	st, releaser, err := h.ctxt.stateForRequestAuthenticatedUser(req)
+	st, err := h.ctxt.stateForRequestAuthenticatedUser(req)
 	if err != nil {
 		h.sendError(resp, err)
 		return
 	}
-	defer releaser()
+	defer st.Release()
 
 	m, err := st.Model()
 	if err != nil {
 		h.sendError(resp, err)
 		return
 	}
 
-	backups, closer := newBackups(st, m)
+	backups, closer := newBackups(st.State, m)
 	defer closer.Close()
 
 	switch req.Method {

diff --git a/apiserver/charms.go b/apiserver/charms.go
@@ -76,7 +76,7 @@ func (h *CharmsHTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 type charmsHandler struct {
 	ctxt          httpContext
 	dataDir       string
-	stateAuthFunc func(*http.Request) (*state.State, state.StatePoolReleaser, error)
+	stateAuthFunc func(*http.Request) (*state.PooledState, error)
 }
 
 // bundleContentSenderFunc functions are responsible for sending a
@@ -92,14 +92,14 @@ func (h *charmsHandler) ServePost(w http.ResponseWriter, r *http.Request) error
 		return errors.Trace(emitUnsupportedMethodErr(r.Method))
 	}
 
-	st, releaser, err := h.stateAuthFunc(r)
+	st, err := h.stateAuthFunc(r)
 	if err != nil {
 		return errors.Trace(err)
 	}
-	defer releaser()
+	defer st.Release()
 
 	// Add a charm to the store provider.
-	charmURL, err := h.processPost(r, st)
+	charmURL, err := h.processPost(r, st.State)
 	if err != nil {
 		return errors.NewBadRequest(err, "")
 	}
@@ -111,18 +111,18 @@ func (h *charmsHandler) ServeGet(w http.ResponseWriter, r *http.Request) error {
 		return errors.Trace(emitUnsupportedMethodErr(r.Method))
 	}
 
-	st, releaser, _, err := h.ctxt.stateForRequestAuthenticated(r)
+	st, _, err := h.ctxt.stateForRequestAuthenticated(r)
 	if err != nil {
 		return errors.Trace(err)
 	}
-	defer releaser()
+	defer st.Release()
 
 	// Retrieve or list charm files.
 	// Requires "url" (charm URL) and an optional "file" (the path to the
 	// charm file) to be included in the query. Optionally also receives an
 	// "icon" query for returning the charm icon or a default one in case the
 	// charm has no icon.
-	charmArchivePath, fileArg, serveIcon, err := h.processGet(r, st)
+	charmArchivePath, fileArg, serveIcon, err := h.processGet(r, st.State)
 	if err != nil {
 		// An error occurred retrieving the charm bundle.
 		if errors.IsNotFound(err) {

diff --git a/apiserver/common/cloudspec/statehelpers.go b/apiserver/common/cloudspec/statehelpers.go
@@ -15,18 +15,18 @@ import (
 // Pool describes an interface for retrieving State instances from a
 // collection.
 type Pool interface {
-	Get(string) (*state.State, state.StatePoolReleaser, error)
+	Get(string) (*state.PooledState, error)
 }
 
 // MakeCloudSpecGetter returns a function which returns a CloudSpec
 // for a given model, using the given Pool.
 func MakeCloudSpecGetter(pool Pool) func(names.ModelTag) (environs.CloudSpec, error) {
 	return func(tag names.ModelTag) (environs.CloudSpec, error) {
-		st, release, err := pool.Get(tag.Id())
+		st, err := pool.Get(tag.Id())
 		if err != nil {
 			return environs.CloudSpec{}, errors.Trace(err)
 		}
-		defer release()
+		defer st.Release()
 
 		m, err := st.Model()
 		if err != nil {
@@ -35,7 +35,9 @@ func MakeCloudSpecGetter(pool Pool) func(names.ModelTag) (environs.CloudSpec, er
 		// TODO - CAAS(externalreality): Once cloud methods are migrated
 		// to model EnvironConfigGetter will no longer need to contain
 		// both state and model but only model.
-		return stateenvirons.EnvironConfigGetter{st, m}.CloudSpec()
+		// TODO (manadart 2018-02-15): This potentially frees the state from
+		// the pool. Release is called, but the state reference survives.
+		return stateenvirons.EnvironConfigGetter{st.State, m}.CloudSpec()
 	}
 }
 

diff --git a/apiserver/common/crossmodel/state.go b/apiserver/common/crossmodel/state.go
@@ -22,18 +22,18 @@ type statePoolShim struct {
 }
 
 func (p *statePoolShim) Get(modelUUID string) (Backend, func(), error) {
-	st, releaser, err := p.StatePool.Get(modelUUID)
+	st, err := p.StatePool.Get(modelUUID)
 	if err != nil {
 		return nil, func() {}, err
 	}
 	closer := func() {
-		releaser()
+		st.Release()
 	}
 	model, err := st.Model()
 	if err != nil {
 		return stateShim{}, closer, err
 	}
-	return stateShim{st, model}, closer, err
+	return stateShim{st.State, model}, closer, err
 }
 
 func GetStatePool(pool *state.StatePool) StatePool {

diff --git a/apiserver/common/modelmanagerinterface.go b/apiserver/common/modelmanagerinterface.go
@@ -141,24 +141,24 @@ func (st modelManagerStateShim) ControllerTag() names.ControllerTag {
 
 // GetBackend implements ModelManagerBackend.
 func (st modelManagerStateShim) GetBackend(modelUUID string) (ModelManagerBackend, func() bool, error) {
-	otherState, release, err := st.pool.Get(modelUUID)
+	otherState, err := st.pool.Get(modelUUID)
 	if err != nil {
 		return nil, nil, errors.Trace(err)
 	}
 	otherModel, err := otherState.Model()
 	if err != nil {
 		return nil, nil, err
 	}
-	return modelManagerStateShim{otherState, otherModel, st.pool}, release, nil
+	return modelManagerStateShim{otherState.State, otherModel, st.pool}, otherState.Release, nil
 }
 
 // GetModel implements ModelManagerBackend.
 func (st modelManagerStateShim) GetModel(modelUUID string) (Model, func() bool, error) {
-	model, release, err := st.pool.GetModel(modelUUID)
+	model, hp, err := st.pool.GetModel(modelUUID)
 	if err != nil {
 		return nil, nil, errors.Trace(err)
 	}
-	return modelShim{model}, release, nil
+	return modelShim{model}, hp.Release, nil
 }
 
 // Model implements ModelManagerBackend.

diff --git a/apiserver/debuglog.go b/apiserver/debuglog.go
@@ -73,12 +73,12 @@ func (h *debugLogHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		socket := &debugLogSocketImpl{conn}
 		defer conn.Close()
 
-		st, releaser, _, err := h.ctxt.stateForRequestAuthenticatedTag(req, names.MachineTagKind, names.UserTagKind)
+		st, _, err := h.ctxt.stateForRequestAuthenticatedTag(req, names.MachineTagKind, names.UserTagKind)
 		if err != nil {
 			socket.sendError(err)
 			return
 		}
-		defer releaser()
+		defer st.Release()
 
 		params, err := readDebugLogParams(req.URL.Query())
 		if err != nil {

diff --git a/apiserver/facades/client/applicationoffers/applicationoffers.go b/apiserver/facades/client/applicationoffers/applicationoffers.go
@@ -67,16 +67,16 @@ func createOffersAPI(
 // NewOffersAPI returns a new application offers OffersAPI facade.
 func NewOffersAPI(ctx facade.Context) (*OffersAPI, error) {
 	environFromModel := func(modelUUID string) (environs.Environ, error) {
-		st, releaser, err := ctx.StatePool().Get(modelUUID)
+		st, err := ctx.StatePool().Get(modelUUID)
 		if err != nil {
 			return nil, errors.Trace(err)
 		}
-		defer releaser()
+		defer st.Release()
 		model, err := st.Model()
 		if err != nil {
 			return nil, errors.Trace(err)
 		}
-		g := stateenvirons.EnvironConfigGetter{st, model}
+		g := stateenvirons.EnvironConfigGetter{st.State, model}
 		env, err := environs.GetEnviron(g, environs.New)
 		if err != nil {
 			return nil, errors.Trace(err)

diff --git a/apiserver/facades/client/applicationoffers/state.go b/apiserver/facades/client/applicationoffers/state.go
@@ -34,22 +34,22 @@ type statePoolShim struct {
 }
 
 func (pool statePoolShim) Get(modelUUID string) (Backend, func(), error) {
-	st, release, err := pool.StatePool.Get(modelUUID)
+	st, err := pool.StatePool.Get(modelUUID)
 	if err != nil {
 		return nil, nil, errors.Trace(err)
 	}
 	return &stateShim{
-		st:      st,
-		Backend: commoncrossmodel.GetBackend(st),
-	}, func() { release() }, nil
+		st:      st.State,
+		Backend: commoncrossmodel.GetBackend(st.State),
+	}, func() { st.Release() }, nil
 }
 
 func (pool statePoolShim) GetModel(modelUUID string) (Model, func(), error) {
-	m, release, err := pool.StatePool.GetModel(modelUUID)
+	m, ph, err := pool.StatePool.GetModel(modelUUID)
 	if err != nil {
 		return nil, nil, errors.Trace(err)
 	}
-	return &modelShim{m}, func() { release() }, nil
+	return &modelShim{m}, func() { ph.Release() }, nil
 }
 
 // Backend provides selected methods off the state.State struct.
@@ -133,14 +133,6 @@ var GetApplicationOffers = func(backend interface{}) crossmodel.ApplicationOffer
 	return nil
 }
 
-type applicationShim struct {
-	*state.Application
-}
-
-func (a *applicationShim) Charm() (ch commoncrossmodel.Charm, force bool, err error) {
-	return a.Application.Charm()
-}
-
 type Subnet interface {
 	CIDR() string
 	VLANTag() int
@@ -220,7 +212,3 @@ func (s *stateShim) User(tag names.UserTag) (User, error) {
 type User interface {
 	DisplayName() string
 }
-
-type userShim struct {
-	*state.User
-}
diff --git a/apiserver/facades/client/client/backend.go b/apiserver/facades/client/client/backend.go
@@ -98,12 +98,12 @@ type stateShim struct {
 	model *state.Model
 }
 
-func (st stateShim) UpdateModelConfig(u map[string]interface{}, r []string, a ...state.ValidateConfigFunc) error {
-	return st.model.UpdateModelConfig(u, r, a...)
+func (s stateShim) UpdateModelConfig(u map[string]interface{}, r []string, a ...state.ValidateConfigFunc) error {
+	return s.model.UpdateModelConfig(u, r, a...)
 }
 
-func (st stateShim) ModelConfigValues() (config.ConfigValues, error) {
-	return st.model.ModelConfigValues()
+func (s stateShim) ModelConfigValues() (config.ConfigValues, error) {
+	return s.model.ModelConfigValues()
 }
 
 func (s *stateShim) Annotations(entity state.GlobalEntity) (map[string]string, error) {
@@ -136,11 +136,11 @@ type poolShim struct {
 }
 
 func (p *poolShim) GetModel(uuid string) (*state.Model, func(), error) {
-	model, release, err := p.pool.GetModel(uuid)
+	model, ph, err := p.pool.GetModel(uuid)
 	if err != nil {
 		return nil, nil, errors.Trace(err)
 	}
-	return model, func() { release() }, nil
+	return model, func() { ph.Release() }, nil
 }
 
 func (s stateShim) ModelConfig() (*config.Config, error) {
@@ -156,6 +156,6 @@ func (s stateShim) ModelConfig() (*config.Config, error) {
 	return cfg, nil
 }
 
-func (st stateShim) ModelTag() names.ModelTag {
-	return names.NewModelTag(st.State.ModelUUID())
+func (s stateShim) ModelTag() names.ModelTag {
+	return names.NewModelTag(s.State.ModelUUID())
 }