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.

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

Switch from zfs get to zfs list calls for better performance #40

Closed
amitkris opened this issue Feb 10, 2016 · 1 comment
Closed

Switch from zfs get to zfs list calls for better performance #40

amitkris opened this issue Feb 10, 2016 · 1 comment

Comments

@amitkris
Copy link
Contributor

amitkris commented Feb 10, 2016

Hi,

This issue is in conjunction with #39.
In an effort to come up with a PR for Solaris support I investigated performance on using zfs list instead of zfs get and came up with the following numbers on Linux.
In short zfs list is quicker, especially when retrieving a specific set of properties.

I wrote a small timer function using the go "time" package that would time zfs Init().
On a Ubuntu system that had 40 containers on it: (As the number of containers on the system increases the time difference also increases drastically)
Without my changes daemon startup showed ZFS init took:

root:~/docker# ./docker-1.11.0-dev-baseline daemon --storage-driver=zfs
ZFS driver init took 716.156429ms time
INFO[0000] Graph migration to content-addressability took 0.00 seconds 
INFO[0000] Firewalld running: false  
INFO[0001] Default bridge (docker0) is assigned with an IP address 172.17.0.0/16. Daemon option --bip can be used to set a preferred IP address 
WARN[0001] Your kernel does not support swap memory limit. 
INFO[0001] Loading containers: start.  
INFO[0002] Loading containers: done.  
INFO[0002] Daemon has completed initialization  
INFO[0002] Docker daemon                                 commit=d687023-unsupported execdriver=native-0.2 graphdriver=zfs version=1.11.0-dev
INFO[0002] API listen on /var/run/docker.sock           

With my changes:

root@:~/docker# bundles/1.11.0-dev/binary/docker-1.11.0-dev daemon --storage-driver=zfs
ZFS driver init took 282.353825ms time
INFO[0000] Graph migration to content-addressability took 0.00 seconds 
INFO[0000] Firewalld running: false  
INFO[0000] Default bridge (docker0) is assigned with an IP address 172.17.0.0/16. Daemon option --bip can be used to set a preferred IP address 
WARN[0001] Your kernel does not support swap memory limit. 
INFO[0001] Loading containers: start.  
INFO[0001] Loading containers: done.  
INFO[0001] Daemon has completed initialization  
INFO[0001] Docker daemon                                 commit=d687023-unsupported execdriver=native-0.2 graphdriver=zfs version=1.11.0-dev
INFO[0001] API listen on /var/run/docker.sock           

Also zfs_test,go does a good job of running all the ZFS commands supported and in all we saved almost 6s on the test run with the changes:

root@:~/vendor/src/github.com/mistifyio/go-zfs# go test
PASS
ok      github.com/mistifyio/go-zfs 20.738s
root@:~/vendor/src/github.com/amitkris/go-zfs# go test
PASS
ok      github.com/amitkris/go-zfs  14.677s

Container startup time is still comparable:

root@:~/docker# time bundles/1.11.0-dev/binary/docker-1.11.0-dev run -i ubuntu /bin/true

real    0m2.368s
user    0m0.104s
sys 0m0.008s
root@:~/docker# time ./docker-1.11.0-dev-baseline run -ti ubuntu /bin/true

real    0m2.039s
user    0m0.076s
sys 0m0.020s

CHANGES made:

  1. Replace zfs/zpool get calls with zfs/zpool list
  2. Specify the list of relevant properties using -o instead of just calling all
  3. Changing parseLine to handle output of list instead of get
  4. Make all the dataset and zpool struct elements strings instead of (strings + integers) because they are being consumed as strings in the graphdriver in Docker anyway( we're going from string->uint64->string, why not just store them as strings..)
amitkris added a commit to amitkris/go-zfs that referenced this issue Feb 10, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Feb 11, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
edillmann pushed a commit to edillmann/go-zfs that referenced this issue Feb 20, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Mar 9, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Mar 10, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Mar 10, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Mar 10, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Mar 28, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Apr 18, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
amitkris added a commit to amitkris/go-zfs that referenced this issue Apr 25, 2016
Enable Solaris support for go-zfs
Switch from zfs/zpool get to zfs/zpool list for better performance
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
mmlb added a commit that referenced this issue Apr 25, 2016
@mmlb
Copy link
Member

mmlb commented Apr 1, 2022

Fixed by #41

@mmlb mmlb closed this as completed Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants