state/presence: Make beings docs smaller #7230

Merged
merged 2 commits into from Apr 12, 2017

Conversation

Projects
None yet
3 participants
Contributor

mjs commented Apr 12, 2017

Description of change

There is no need to stored the model-uuid separately as it's already
in the _id prefix. The query on model-uuid can be replaced with a
prefix match on _id (this uses the index on _id).

Given that many beings documents are created this as a significant
impact on disk space usage.

QA steps

Bootstrapped a controller with a few units and models and confirmed that presence still functions. Confirmed new log message works like this:

$ juju model-config -m controller logging-config="<root>=INFO;juju=DEBUG;unit=DEBUG;juju.state.presence=TRACE"
$ juju debug-log --include-module juju.state.presence --replay -m controller
machine-0: 17:37:04 TRACE juju.state.presence [1004c4] loaded 0 beings in 409.522µs
...
machine-0: 17:37:24 TRACE juju.state.presence [5e88ac] loaded 5 beings in 879.482µs
...
machine-0: 17:37:24 TRACE juju.state.presence [5e88ac] loaded 5 beings in 3.432866ms
...

Documentation changes

N.A. (internal only)

Bug reference

Contributes to fixing https://bugs.launchpad.net/juju/+bug/1454661

I think this is good as is if we just wanted to land it. I have some thoughts about things we could improve, and I wonder if we want to add an "Upgrade" step to remove it from active databases. But I think you mentioned an upgrade step that just drops everything in presence anyway, which would certainly have the desired effect.

state/presence/presence.go
+ beings := make([]beingInfo, 0)
+ err := beingsC.Find(bson.M{
+ "_id": bson.M{"$regex": bson.RegEx{"^" + w.modelUUID, ""}},
+ }).All(&beings)
@jameinel

jameinel Apr 12, 2017

Owner

given this table is larger than we think it is, should we be actually using BatchSize and Iter() to load it into the map?

state/presence/presence.go
if err != nil {
return nil, err
}
+ dt := time.Now().Sub(t0)
+ logger.Tracef("[%s] loaded %d beings in %s", w.modelUUID[:6], len(beings), dt)
@jameinel

jameinel Apr 12, 2017

Owner

this feels like it might be worthy for Debug. Trace is always something that "you have to know it exists to turn it on", whereas "Debug" is "would it be useful to get this sort of information from the field".

mjs added some commits Apr 12, 2017

state/presence: Make beings docs smaller
There is no need to stored the model-uuid separately as it's already
in the _id prefix. The query on model-uuid can be replaced with a
prefix match on _id. This uses the index on _id speeding up the
findAllBeings query.

Given that many beings documents are created this as a significant
impact on disk space usage.

Part of the fix for https://bugs.launchpad.net/juju/+bug/1454661
state/presence: Use iterator when loading all beings
In a long running deployment with many entities, the beings collection
can be large. Use an iterator to avoid the large and temporary slice.
Contributor

mjs commented Apr 12, 2017

$$merge$$

Contributor

jujubot commented Apr 12, 2017

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

Contributor

jujubot commented Apr 12, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10667

Contributor

mjs commented Apr 12, 2017

$$unrelated-lxd-weirdness$$

Contributor

jujubot commented Apr 12, 2017

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

@jujubot jujubot merged commit fac9c45 into juju:develop Apr 12, 2017

1 check failed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details

jujubot added a commit that referenced this pull request Jun 2, 2017

Merge pull request #7443 from axw/2.1-backport-performance-fixes
Backport performance fixes to 2.1

Backport improvements to presence queries and pruning, and status history pruning, to 2.1 from develop.

Includes:
 - #7230
 - #7248
 - #7080

@mjs mjs deleted the mjs:1454661-presence-smaller-beings branch Jul 25, 2017

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