Skip to content

Commit

Permalink
Merge pull request #7512 from mjs/1698701-AllModelWatcher-ForModel-2.2
Browse files Browse the repository at this point in the history
state: Use StatePool instead of ForModel in AllModelWatcher

## Description of change

ForModel is inefficient. Starting a AllModelWatcher on a controller with many models would have had a significant resource impact.

Also a drive-by fix for an incorrect call in state/logdb.

## QA steps

Bootstrapped a new controller and then connected using this client: https://gist.github.com/mjs/d9d016433e4348b4aa990a8f81d65d03

Deployed software in multiple models and ensured that all expected changes were observed.

## Documentation changes

N.A.

## Bug reference

https://bugs.launchpad.net/juju/+bug/1698701
  • Loading branch information
jujubot committed Jun 19, 2017
2 parents 97aa24d + 51c6f35 commit 725099c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
9 changes: 9 additions & 0 deletions apiserver/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
type controllerSuite struct {
statetesting.StateSuite

statePool *state.StatePool
controller *controller.ControllerAPI
resources *common.Resources
authorizer apiservertesting.FakeAuthorizer
Expand All @@ -50,6 +51,13 @@ func (s *controllerSuite) SetUpTest(c *gc.C) {
})

s.StateSuite.SetUpTest(c)

s.statePool = state.NewStatePool(s.State)
s.AddCleanup(func(c *gc.C) {
err := s.statePool.Close()
c.Assert(err, jc.ErrorIsNil)
})

s.resources = common.NewResources()
s.AddCleanup(func(_ *gc.C) { s.resources.StopAll() })

Expand All @@ -61,6 +69,7 @@ func (s *controllerSuite) SetUpTest(c *gc.C) {
controller, err := controller.NewControllerAPI(
facadetest.Context{
State_: s.State,
StatePool_: s.statePool,
Resources_: s.resources,
Auth_: s.authorizer,
})
Expand Down
4 changes: 2 additions & 2 deletions state/allwatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -1330,11 +1330,11 @@ func (b *allModelWatcherStateBacking) GetAll(all *multiwatcherStore) error {
}

func (b *allModelWatcherStateBacking) loadAllWatcherEntitiesForModel(m *Model, all *multiwatcherStore) error {
st, err := b.st.ForModel(m.ModelTag())
st, releaser, err := b.stPool.Get(m.UUID())
if err != nil {
return errors.Trace(err)
}
defer st.Close()
defer releaser()

err = loadAllWatcherEntities(st, b.collectionByName, all)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion state/logdb/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (s *BufferedLoggerSuite) assertNoFlush(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
select {
case records := <-s.mock.called:
c.Fatal("unexpected log records: %v", records)
c.Fatalf("unexpected log records: %v", records)
case <-time.After(coretesting.ShortWait):
}
}
Expand Down

0 comments on commit 725099c

Please sign in to comment.