8000 fleetd: get filtered list of units instead of all units by kayrus · Pull Request #1564 · coreos/fleet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

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

Merged
merged 1 commit into from
Jun 27, 2016
Merged

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

merged 1 commit into from
Jun 27, 2016

Conversation

kayrus
Copy link
Contributor
@kayrus kayrus commented Apr 21, 2016 •
edited
Loading

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

This was referenced Apr 21, 2016
Merged
Closed
@kayrus
Copy link
Contributor Author
kayrus commented Apr 22, 2016

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" ""
# hello@3.service
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties GetAll "s" ""
# hello@3.service
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties Get "ss" "org.freedesktop.systemd1.Unit" "LoadState"
# hello@3.service
busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/hello_403_2eservice \
org.freedesktop.DBus.Properties GetAll "s" "org.freedesktop.systemd1.Unit"

@kayrus
Copy link
Contributor Author
kayrus commented Apr 26, 2016 •
edited
Loading

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 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
Merged
Closed
@kayrus kayrus force-pushed the kayrus/list_filtered_units branch from eaaae32 to 58ccffc Compare May 3, 2016 15:07
@kayrus
Copy link
Contributor Author
kayrus commented May 3, 2016

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

@kayrus kayrus force-pushed the kayrus/list_filtered_units branch from 58ccffc to a90aa38 Compare May 3, 2016 15:20
@kayrus kayrus force-pushed the kayrus/list_filtered_units branch from a90aa38 to de73981 Compare May 11, 2016 16:09
@kayrus
Copy link
Contributor Author
kayrus commented May 11, 2016

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

@jonboulle jonboulle added this to the v1.0.0 milestone May 18, 2016
@kayrus kayrus force-pushed the kayrus/list_filtered_units branch 4 times, most recently from 1ca90cb to 7271d4a Compare May 24, 2016 12:49
@kayrus
Copy link
Contributor Author
kayrus commented May 25, 2016

Test case:

One template unit: hello@.service

[Service]
Type=oneshot
RemainAfterExit=true
ExecStart=/bin/true
fleetctl submit hello@.service
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
Copy link
Contributor Author
kayrus commented May 25, 2016

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

jonboulle
jonboulle reviewed May 25, 2016
View reviewed changes
systemd/manager.go
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, ignore this, you're right

@kayrus kayrus force-pushed the kayrus/list_filtered_units branch 2 times, most recently from e0b2094 to 94a8001 Compare May 31, 2016 14:43
@kayrus kayrus force-pushed the kayrus/list_filtered_units branch from 94a8001 to b96d05f Compare June 7, 2016 16:29
@kayrus 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
@kayrus
Copy link
Contributor Author
kayrus commented Jun 7, 2016

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

@kayrus
fleetd: implemented GetUnitsByNames dbus method
4edc9ba 
* Updated vendor, set go-systemd to v9
@kayrus kayrus force-pushed the kayrus/list_filtered_units branch from b96d05f to 4edc9ba Compare June 7, 2016 16:35
@kayrus
Copy link
Contributor Author
kayrus commented Jun 7, 2016

@hectorj2f ping

@hectorj2f
Copy link
Contributor

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

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

LGTM. I'll merge it soon.

@hectorj2f
Copy link
Contributor

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

@dongsupark dongsupark merged commit fdf1dd9 into coreos:master Jun 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
area/performance dependency/external kind/enhancement priority/Pmaybe reviewed/lgtm technology/systemd
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.

filter ListUnits dbus method to subscribed units
4 participants
@kayrus @hectorj2f @dongsupark @jonboulle
0