Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

state/presence: cleanups for issue 1588574 #5530

Closed
wants to merge 1 commit into from
Closed

state/presence: cleanups for issue 1588574 #5530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions state/presence/presence.go
View file
Open in desktop
Expand Up @@ -696,13 +696,6 @@ func (p *Pinger) prepare() error {
// sequence in use by the pinger.
func (p *Pinger) ping() (err error) {
logger.Tracef("pinging %q with seq=%d", p.beingKey, p.beingSeq)
defer func() {
// If the session is killed from underneath us, it panics when we
// try to copy it, so deal with that here.
if v := recover(); v != nil {
err = fmt.Errorf("%v", v)
}
}()
session := p.pings.Database.Session.Copy()
defer session.Close()
if p.delta == 0 {
Expand Down
58 changes: 34 additions & 24 deletions state/presence/presence_test.go
View file
Open in desktop
Expand Up @@ -111,22 +111,32 @@ func (s *PresenceSuite) TestErrAndDead(c *gc.C) {
}
}

// assertStopped stops a worker and waits until it reports stopped.
// Use this method in favor of defer w.Stop() because you _must_ ensure
// that the worker has stopped, and thus is no longer using its mgo
// session before TearDownTest shuts down the connection.
func assertStopped(c *gc.C, w worker.Worker) {
c.Assert(w.Stop(), gc.IsNil)
w.Wait()
}

func (s *PresenceSuite) TestAliveError(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
c.Assert(w.Stop(), gc.IsNil)

alive, err := w.Alive("a")
c.Assert(err, gc.ErrorMatches, ".*: watcher is dying")
c.Assert(alive, jc.IsFalse)
w.Wait()
}

func (s *PresenceSuite) TestWorkflow(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
pa := presence.NewPinger(s.presence, s.modelTag, "a")
pb := presence.NewPinger(s.presence, s.modelTag, "b")
defer w.Stop()
defer pa.Stop()
defer pb.Stop()
defer assertStopped(c, w)
defer assertStopped(c, pa)
defer assertStopped(c, pb)

assertAlive(c, w, "a", false)
assertAlive(c, w, "b", false)
Expand Down Expand Up @@ -194,7 +204,7 @@ func (s *PresenceSuite) TestWorkflow(c *gc.C) {
assertChange(c, cha, presence.Change{"a", false})
assertChange(c, chb, presence.Change{"b", false})

c.Assert(w.Stop(), gc.IsNil)
assertStopped(c, w)
}

func (s *PresenceSuite) TestScale(c *gc.C) {
Expand All @@ -220,7 +230,7 @@ func (s *PresenceSuite) TestScale(c *gc.C) {

c.Logf("Checking who's still alive...")
w := presence.NewWatcher(s.presence, s.modelTag)
defer w.Stop()
defer assertStopped(c, w)
w.Sync()
ch := make(chan presence.Change)
for i := 0; i < N; i++ {
Expand All @@ -237,8 +247,8 @@ func (s *PresenceSuite) TestScale(c *gc.C) {
func (s *PresenceSuite) TestExpiry(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
p := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p.Stop()
defer assertStopped(w, c)
defer assertStopped(w, p)

ch := make(chan presence.Change)
w.Watch("a", ch)
Expand Down Expand Up @@ -270,8 +280,8 @@ func (s *PresenceSuite) TestWatchPeriod(c *gc.C) {

w := presence.NewWatcher(s.presence, s.modelTag)
p := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p.Stop()
defer assertStopped(c, w)
defer assertStopped(c, p)

ch := make(chan presence.Change)
w.Watch("a", ch)
Expand Down Expand Up @@ -315,7 +325,7 @@ func (s *PresenceSuite) TestWatchUnwatchOnQueue(c *gc.C) {
func (s *PresenceSuite) TestRestartWithoutGaps(c *gc.C) {
p := presence.NewPinger(s.presence, s.modelTag, "a")
c.Assert(p.Start(), gc.IsNil)
defer p.Stop()
defer assertStopped(c, p)

done := make(chan bool)
go func() {
Expand All @@ -339,7 +349,7 @@ func (s *PresenceSuite) TestRestartWithoutGaps(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
w.Sync()
alive, err := w.Alive("a")
c.Check(w.Stop(), gc.IsNil)
assertStopped(c, w)
if !c.Check(err, jc.ErrorIsNil) || !c.Check(alive, jc.IsTrue) {
break
}
Expand Down Expand Up @@ -369,9 +379,9 @@ func (s *PresenceSuite) TestPingerPeriodAndResilience(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
p1 := presence.NewPinger(s.presence, s.modelTag, "a")
p2 := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p1.Stop()
defer p2.Stop()
defer assertStopped(c, w)
defer assertStopped(c, p1)
defer assertStopped(c, p2)

// Start p1 and let it go on.
c.Assert(p1.Start(), gc.IsNil)
Expand Down Expand Up @@ -399,8 +409,8 @@ func (s *PresenceSuite) TestPingerPeriodAndResilience(c *gc.C) {
func (s *PresenceSuite) TestStartSync(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
p := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p.Stop()
defer assertStopped(c, w)
defer assertStopped(c, p)

ch := make(chan presence.Change)
w.Watch("a", ch)
Expand Down Expand Up @@ -428,8 +438,8 @@ func (s *PresenceSuite) TestStartSync(c *gc.C) {
func (s *PresenceSuite) TestSync(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
p := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p.Stop()
defer assertStopped(c, w)
defer assertStopped(c, p)

ch := make(chan presence.Change)
w.Watch("a", ch)
Expand Down Expand Up @@ -467,8 +477,8 @@ func (s *PresenceSuite) TestSync(c *gc.C) {
func (s *PresenceSuite) TestFindAllBeings(c *gc.C) {
w := presence.NewWatcher(s.presence, s.modelTag)
p := presence.NewPinger(s.presence, s.modelTag, "a")
defer w.Stop()
defer p.Stop()
defer assertStopped(c, w)
defer assertStopped(c, p)

ch := make(chan presence.Change)
w.Watch("a", ch)
Expand All @@ -493,12 +503,12 @@ func (s *PresenceSuite) TestFindAllBeings(c *gc.C) {
func (s *PresenceSuite) TestTwoEnvironments(c *gc.C) {
key := "a"
w1, p1, ch1 := s.setup(c, key)
defer w1.Stop()
defer p1.Stop()
defer assertStopped(c, w1)
defer assertStopped(c, p1)

w2, p2, ch2 := s.setup(c, key)
defer w2.Stop()
defer p2.Stop()
defer assertStopped(c, w2)
defer assertStopped(c, p2)

c.Assert(p1.Start(), gc.IsNil)
w1.StartSync()
Expand Down