/juju

Clean up presence tables on model destruction. #7260

Merged
merged 2 commits into from Apr 21, 2017

Conversation

Reviewers

@mjs mjs

@wupeka wupeka

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@jameinel @wupeka @jujubot @mjs
@jameinel
Owner

jameinel commented Apr 20, 2017 •
Edited 1 time

Description of change

It seems we were destroying Logs but not Presence.
This just adds a helper to the presence package for
removing everything for a given model UUID and then
updates the state destruction code to call it.

QA steps

  $ juju bootstrap
  $ for i in `seq 10`;  do 
    juju add-model model-$i; 
    juju deploy ubuntu;
    juju add-unit ubuntu -n 9 --to 0,0,0,0,0,0,0,0,0
    # wait
    juju destroy-model -y model-$i
  done

At this point, the presence collection should not be very big, it should only have records for the controller model. It should not have any beings, seqs, or pings for any of the units that were created in the middle.

Documentation changes

No, this is an internal cleanup.

Bug reference

It was realized as part of bug #1454661 though it is not directly that bug.

jameinel added some commits Apr 20, 2017

@jameinel
Clean up presence tables on model destruction. 
It seems we were destroying Logs but not Presence.
This just adds a helper to the presence package for
removing everything for a given model UUID and then
updates the state destruction code to call it.
e99d8b0
@jameinel
go fmt and add logging of items removed
8f157e5
@wupeka
Member

wupeka commented Apr 20, 2017

Looks OK to me.
@jameinel
Owner

jameinel commented Apr 21, 2017

@jujubot
Contributor

jujubot commented Apr 21, 2017

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

@jujubot jujubot merged commit a201b3e into juju:develop Apr 21, 2017

state/presence/presence.go
+
+// RemovePresenceForModel removes all of the records of entities for a given model
+// across all of the collections.
+func RemovePresenceForModel(base *mgo.Collection, modelTag names.ModelTag) error {
@mjs

mjs Apr 21, 2017

Contributor

Why does this take a tag and not a model UUID? Tags are primarily for the API right? That tag is only being used to extract the model UUID.

@jameinel

jameinel Apr 21, 2017

Owner

Other parts of Presence also take names.ModelTag (NewPinger and NewWatcher)
At one point "tags" were certainly just for the API. From reading the code I think the idea is to have "typed strings" so that you don't pass a machine Id vs a unit vs a model. Otherwise they are all just strings.

@jameinel jameinel deleted the jameinel:2.2-prune-presence-model-destruction branch Apr 22, 2017

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