fleetd: get filtered list of units instead of all units #1564

kayrus · 2016-04-21T15:14:50Z

resolves #478. Code works with these systemd changes: endocode/systemd@5b4452a
Otherwise fleet will fallback to the normal ListUnits method.

I don't like this code: https://github.com/endocode/fleet/blob/9828ed19752bdaae167d79e458fd7200ca3cb305/systemd/manager.go#L248

Instead of calling several times for each unit's status I'd like to implement new method called GetUnitsProperties

This PR contains godeps diffs only for test purposes. These changes are also introduced in go-systemd repo: coreos/go-systemd#150

/cc @jonboulle @tixxdz @antrik

kayrus · 2016-04-22T10:01:20Z

Just for the history. Commands to test systemd dbus methods using busctl:

# fleet.service
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/fleet_2eservice \
org.freedesktop.DBus.Properties GetAll "s" ""
# [email protected]
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties GetAll "s" ""
# [email protected]
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties Get "ss" "org.freedesktop.systemd1.Unit" "LoadState"
# [email protected]
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties GetAll "s" "org.freedesktop.systemd1.Unit"

kayrus · 2016-04-26T14:49:23Z

Got a bug, systemctl -M smokeccc9d3b3714a4db0a9e7197499f0323 status polkit.service always prints unit status independently on machine name.

UPD: Hm, have noticed same result with vanilla v225 systemd branch.

UPD2: Machine filter was not applied because of the patch which was reverted in coreos branch: coreos/systemd#27

kayrus · 2016-05-03T15:07:06Z

Have create yet another systemd improvement: systemd/systemd#3182
Here is the PR for go-systemd: coreos/go-systemd#155

kayrus · 2016-05-11T16:10:33Z

rebased and updated code. Now all units are retrieved by ListUnitsByNames dbus method which was implemented here: systemd/systemd#3182

kayrus · 2016-05-25T10:12:48Z

Test case:

One template unit: [email protected]

[Service]
Type=oneshot
RemainAfterExit=true
ExecStart=/bin/true

fleetctl submit [email protected]
fleetctl load hello@{1..500}

And we have 500 inactive units which triggers exactly the same issue which was reported in the #1418.

With systemd < 230:

500 units info retrieving takes 7-8 seconds when you use systemd < 230.
cpu load for fleet/systemd - 35-55/30-50 %

top output for 60 secs:

$ top -d 60
top - 10:11:17 up 6 min,  3 users,  load average: 1.47, 1.31, 0.62
Tasks:  67 total,   1 running,  66 sleeping,   0 stopped,   0 zombie
%Cpu(s): 66.8 us, 31.8 sy,  0.0 ni,  0.0 id,  0.0 wa,  1.1 hi,  0.3 si,  0.0 st
KiB Mem:    505052 total,   310236 used,   194816 free,     9320 buffers
KiB Swap:        0 total,        0 used,        0 free.   165880 cached Mem

  PID USER      PR  NI    VIRT    RES    SHR S %CPU %MEM     TIME+ COMMAND                                              
  748 fleet     20   0   53952  51332   6768 S 47.4 10.2   3:02.35 fleetd                                               
    1 root      20   0  119576   6252   4480 S 41.4  1.2   1:17.40 systemd                                              
  610 message+  20   0   42632   3876   3164 S  5.8  0.8   0:11.24 dbus-daemon                                          
  739 etcd      20   0  322800  48168  13612 S  3.3  9.5   0:47.10 etcd2

With the systemd 230:

cpu load for fleet/systemd - 30-40/15-20 %
499 units info retrieving takes less than 1 second

top output for 60 secs:

$ top -d 60
top - 10:03:40 up 11 min,  3 users,  load average: 0.19, 0.54, 0.44
Tasks:  68 total,   3 running,  65 sleeping,   0 stopped,   0 zombie
%Cpu(s): 52.3 us,  7.2 sy,  0.0 ni, 38.9 id,  0.6 wa,  0.6 hi,  0.2 si,  0.2 st
KiB Mem:    505052 total,   297516 used,   207536 free,     9764 buffers
KiB Swap:        0 total,        0 used,        0 free.   177768 cached Mem

  PID USER      PR  NI    VIRT    RES    SHR S %CPU %MEM     TIME+ COMMAND                                              
  929 root      20   0   25172  20120   6008 S 33.7  4.0   2:11.35 fleetd                                               
    1 root      20   0  105960   6832   4152 R 18.3  1.4   1:36.35 systemd                                              
  717 etcd      20   0  336136  53012  13364 S  3.5 10.5   0:58.63 etcd2                                                
  563 message+  20   0   34384   3776   3100 R  2.8  0.7   0:15.54 dbus-daemon

As a result we have 40-50% advantage for systemd performance, for fleetd: 15-30%.

kayrus · 2016-05-25T10:19:45Z

@hectorj2f can you benchmark this PR with the latest systemd 230?

jonboulle · 2016-05-25T15:23:39Z

systemd/manager.go

 	states := make(map[string]*unit.UnitState)
 	for _, dus := range dbusStatuses {
-		if !filter.Contains(dus.Name) {
+		if fallback && !filter.Contains(dus.Name) {


Why are you going through this loop even if the filtered call worked?

sorry, ignore this, you're right

kayrus · 2016-06-07T16:30:30Z

go-systemd was updated. I removed [WIP] and updated this PR.

* Updated vendor, set go-systemd to v9

kayrus · 2016-06-07T18:00:12Z

@hectorj2f ping

hectorj2f · 2016-06-07T18:06:10Z

@kayrus Nice. I'll benchmark it to find the benefits.

dongsupark · 2016-06-27T08:33:37Z

LGTM. I'll merge it soon.

hectorj2f · 2016-06-27T08:45:05Z

LGTM, I didn't bake any plots yet. But it works smoothly and a bit faster than the current version.

This was referenced Apr 21, 2016

dbus: add support for new ListUnits* methods coreos/go-systemd#150

Merged

Add unit state cache to mitigate the iteration between fleet and systemd - dbus #1418

Closed

kayrus force-pushed the kayrus/list_filtered_units branch from 9828ed1 to fcd97f0 Compare April 29, 2016 14:03

This was referenced May 3, 2016

core: added ListUnitsByNames dbus method systemd/systemd#3182

Merged

[WIP] dbus: implemented GetUnitsStates method coreos/go-systemd#155

Closed

kayrus force-pushed the kayrus/list_filtered_units branch from eaaae32 to 58ccffc Compare May 3, 2016 15:07

kayrus force-pushed the kayrus/list_filtered_units branch from 58ccffc to a90aa38 Compare May 3, 2016 15:20

dongsupark added kind/enhancement technology/systemd priority/Pmaybe needs review area/performance dependency/external labels May 9, 2016

kayrus force-pushed the kayrus/list_filtered_units branch from a90aa38 to de73981 Compare May 11, 2016 16:09

jonboulle added this to the v1.0.0 milestone May 18, 2016

dongsupark force-pushed the master branch from 0338168 to 56b539f Compare May 23, 2016 17:57

kayrus force-pushed the kayrus/list_filtered_units branch 4 times, most recently from 1ca90cb to 7271d4a Compare May 24, 2016 12:49

jonboulle reviewed May 25, 2016
View reviewed changes

kayrus force-pushed the kayrus/list_filtered_units branch 2 times, most recently from e0b2094 to 94a8001 Compare May 31, 2016 14:43

kayrus force-pushed the kayrus/list_filtered_units branch from 94a8001 to b96d05f Compare June 7, 2016 16:29

kayrus changed the title ~~[WIP] fleetd: get filtered list of units instead of all units~~ fleetd: get filtered list of units instead of all units Jun 7, 2016

fleetd: implemented GetUnitsByNames dbus method

4edc9ba

* Updated vendor, set go-systemd to v9

kayrus force-pushed the kayrus/list_filtered_units branch from b96d05f to 4edc9ba Compare June 7, 2016 16:35

dongsupark force-pushed the master branch 2 times, most recently from 14580b0 to 63b20dc Compare June 22, 2016 10:22

dongsupark added reviewed/lgtm and removed needs review labels Jun 27, 2016

dongsupark merged commit fdf1dd9 into coreos:master Jun 27, 2016

fleetd: get filtered list of units instead of all units #1564

fleetd: get filtered list of units instead of all units #1564

Uh oh!

Conversation

kayrus commented Apr 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kayrus commented Apr 22, 2016

Uh oh!

kayrus commented Apr 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kayrus commented May 3, 2016

Uh oh!

kayrus commented May 11, 2016

Uh oh!

kayrus commented May 25, 2016

With systemd < 230:

With the systemd 230:

Uh oh!

kayrus commented May 25, 2016

Uh oh!

jonboulle May 25, 2016

Choose a reason for hiding this comment

Uh oh!

jonboulle May 25, 2016

Choose a reason for hiding this comment

Uh oh!

kayrus commented Jun 7, 2016

Uh oh!

kayrus commented Jun 7, 2016

Uh oh!

hectorj2f commented Jun 7, 2016

Uh oh!

dongsupark commented Jun 27, 2016

Uh oh!

hectorj2f commented Jun 27, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kayrus commented Apr 21, 2016 •

edited

Loading

kayrus commented Apr 26, 2016 •

edited

Loading