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

Add gcepd support for VolumeInspectByName #579

Merged

Conversation

codenrhoden
Copy link
Contributor

Implement VolumeInspectByName for the gcepd driver. This provides an optimized lookup when callers are wanting to lookup a volume by name instead of by ID. This prevents the framework (>= 0.6.1) or the integration driver (<= v0.6.0 and before from having to a Volumes() call and then string match on the name.

Just for fun, some simple performance numbers. This is using REX-Ray 0.9.0, 0.9.1, and 0.9.1+this patch on GCE with 10 volumes defined. Ran a loop 10 times with docker inspect. REX-Ray 0.9 would have called Volumes(), retrieving a list of all volumes and then finding the desired one to inspect. 0.9.1 Would call VolumeInspect() with a flag that has the framework do the Volumes() call followed by the string match instead of the integration driver (eliminating an entire API call between client and server, but it still occurs from the framework so there isn't a big different when doing all-in-one), and this patch would have the framework call VolumeInsepectByName() directly, eliminating the call to Volumes() entirely.

0.9.1+ 0.9.1 0.9.0
real 0m0.211s real 0m0.296s real 0m0.281s
real 0m0.233s real 0m0.386s real 0m0.253s
real 0m0.202s real 0m0.293s real 0m0.288s
real 0m0.234s real 0m0.269s real 0m0.270s
real 0m0.204s real 0m0.286s real 0m0.278s
real 0m0.190s real 0m0.313s real 0m0.281s
real 0m0.203s real 0m0.284s real 0m0.273s
real 0m0.199s real 0m0.351s real 0m0.314s
real 0m0.356s real 0m0.341s real 0m0.319s
real 0m0.245s real 0m0.294s real 0m0.300s

This results in an average (in seconds) of:

0.9.1+ 0.9.1 0.9.0
0.2277 0.3113 0.2857

As expected, there isn't much difference between 0.9.0 and 0.9.1 in this setup. Though I think 0.9.1 would win out if you scaled up the number of volumes and made it a distributed setup. But this patch has the biggest effect overall, because no call to Volumes() is needed at all. DIsparity between the two will only grow as the number of volumes increases.

FWIW, this performance improvement is only realizable by Docker and docker volume commands right now. Some changes would have to occur in REX-Ray to take advantage of this from the rexray CLI.

@codenrhoden codenrhoden added this to the 2017.06-2 milestone Jun 21, 2017
@codecov-io
Copy link

Codecov Report

Merging #579 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #579   +/-   ##
=======================================
  Coverage   28.42%   28.42%           
=======================================
  Files          34       34           
  Lines        2016     2016           
=======================================
  Hits          573      573           
  Misses       1381     1381           
  Partials       62       62

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb8a4bc...f30c7cf. Read the comment docs.

@akutz akutz self-requested a review June 26, 2017 17:41
@akutz akutz self-assigned this Jun 26, 2017
@akutz akutz merged commit 0966339 into thecodeteam:master Jun 26, 2017
@codenrhoden codenrhoden deleted the feature/gcepd-inspect-vol-name branch June 26, 2017 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants