Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Yaniv Bronhaim has submitted this change and it was merged. Change subject: fileSD: Optimize getAllVolumes on file storage .. fileSD: Optimize getAllVolumes on file storage The previous implementation was using O(N^2) search to detect template images and volumes. When working with storage domain with large number of disks created from the same template, getAllVolumes becomes very slow, delaying flows such as vm startup and recovery. This patch reimplements getAllVolumes using O(N) search, cutting the time to process 5000 volume paths from 9 to 0.065 seconds. Profiling on scale setup show that getAllVolume cpu time per call dropped from 44 to 1.3 seconds, and the total cpu time used for preparing 20 images drop from 884 to 28 seconds. Before this patch: ncalls tottime percall cumtime percall filename:lineno(function) ... 200.1530.008 884.109 44.205 hsm.py:3188(HSM.prepareImage) 207.2420.362 883.014 44.151 fileSD.py:414(NfsStorageDomain.getAllVolumes) 214840 866.0800.004 866.0800.004 fileSD.py:444(genexpr) ... With this patch: ncalls tottime percall cumtime percall filename:lineno(function) ... 200.1280.006 28.3021.415 hsm.py:3188(HSM.prepareImage) 20 10.3910.520 26.5791.329 fileSD.py:415(NfsStorageDomain.getAllVolumes) ... Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Bug-Url: https://bugzilla.redhat.com/1223053 Signed-off-by: Nir Soffer nsof...@redhat.com Reviewed-on: http://gerrit.ovirt.org/36593 Reviewed-by: Allon Mureinik amure...@redhat.com Reviewed-by: Adam Litke ali...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com Reviewed-by: Dan Kenigsberg dan...@redhat.com Reviewed-on: https://gerrit.ovirt.org/41127 Continuous-Integration: Jenkins CI Reviewed-by: Eldad Marciano emarc...@redhat.com Tested-by: Eldad Marciano emarc...@redhat.com Reviewed-by: Yaniv Bronhaim ybron...@redhat.com --- A tests/fileSDTests.py M vdsm/storage/fileSD.py 2 files changed, 205 insertions(+), 26 deletions(-) Approvals: Yaniv Bronhaim: Looks good to me, approved Jenkins CI: Passed CI tests Allon Mureinik: Looks good to me, but someone else must approve Eldad Marciano: Verified; Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eldad Marciano emarc...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
automat...@ovirt.org has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 2: * Update tracker::#1223053::OK * Set MODIFIED::bug 1223053#1223053IGNORE, not oVirt prod but Red Hat Enterprise Virtualization Manager -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eldad Marciano emarc...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Nir Soffer has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: Thanks for testing this Eldad! Can you describe how you tested this patch? What kind of configuration you tested (storage, number of disk, templates etc.), and which flows did you test? Also how does the patch effect the system behavior? in your tests? -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eldad Marciano emarc...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Eldad Marciano has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: Verified+1 Code-Review+1 verified on top of vt15.1 -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eldad Marciano emarc...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Yaniv Bronhaim has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eldad Marciano emarc...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Allon Mureinik has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: Yaniv/Dan? Can we move forward here? -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Allon Mureinik has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: Code-Review+1 Can we move forward with this one please? -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
Hello Adam Litke, Dan Kenigsberg, Allon Mureinik,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/41127
to review the following change.
Change subject: fileSD: Optimize getAllVolumes on file storage
..
fileSD: Optimize getAllVolumes on file storage
The previous implementation was using O(N^2) search to detect template
images and volumes. When working with storage domain with large number
of disks created from the same template, getAllVolumes becomes very
slow, delaying flows such as vm startup and recovery.
This patch reimplements getAllVolumes using O(N) search, cutting the
time to process 5000 volume paths from 9 to 0.065 seconds.
Profiling on scale setup show that getAllVolume cpu time per call
dropped from 44 to 1.3 seconds, and the total cpu time used for
preparing 20 images drop from 884 to 28 seconds.
Before this patch:
ncalls tottime percall cumtime percall filename:lineno(function)
...
200.1530.008 884.109 44.205 hsm.py:3188(HSM.prepareImage)
207.2420.362 883.014 44.151
fileSD.py:414(NfsStorageDomain.getAllVolumes)
214840 866.0800.004 866.0800.004 fileSD.py:444(genexpr)
...
With this patch:
ncalls tottime percall cumtime percall filename:lineno(function)
...
200.1280.006 28.3021.415 hsm.py:3188(HSM.prepareImage)
20 10.3910.520 26.5791.329
fileSD.py:415(NfsStorageDomain.getAllVolumes)
...
Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece
Bug-Url: https://bugzilla.redhat.com/1223053
Signed-off-by: Nir Soffer nsof...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/36593
Reviewed-by: Allon Mureinik amure...@redhat.com
Reviewed-by: Adam Litke ali...@redhat.com
Reviewed-by: Federico Simoncelli fsimo...@redhat.com
Reviewed-by: Dan Kenigsberg dan...@redhat.com
---
A tests/fileSDTests.py
M vdsm/storage/fileSD.py
2 files changed, 205 insertions(+), 26 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/41127/1
diff --git a/tests/fileSDTests.py b/tests/fileSDTests.py
new file mode 100644
index 000..39df65a
--- /dev/null
+++ b/tests/fileSDTests.py
@@ -0,0 +1,157 @@
+#
+# Copyright 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import fnmatch
+import os
+import time
+import uuid
+
+from testrunner import VdsmTestCase as TestCaseBase
+
+from storage import fileSD
+from storage import sd
+
+
+class TestingFileStorageDomain(fileSD.FileStorageDomain):
+
+stat = None # Accessed in __del__
+
+def __init__(self, uuid, mountpoint, oop):
+self.sdUUID = uuid
+self.mountpoint = mountpoint
+self._oop = oop
+
+@property
+def oop(self):
+return self._oop
+
+
+class FakeGlob(object):
+
+def __init__(self, files):
+self.files = files
+
+def glob(self, pattern):
+return fnmatch.filter(self.files, pattern)
+
+
+class FakeOOP(object):
+
+def __init__(self, glob=None):
+self.glob = glob
+
+
+class GetAllVolumesTests(TestCaseBase):
+
+MOUNTPOINT = /rhev/data-center/%s % uuid.uuid4()
+SD_UUID = str(uuid.uuid4())
+IMAGES_DIR = os.path.join(MOUNTPOINT, SD_UUID, sd.DOMAIN_IMAGES)
+
+def test_no_volumes(self):
+oop = FakeOOP(FakeGlob([]))
+dom = TestingFileStorageDomain(self.SD_UUID, self.MOUNTPOINT, oop)
+res = dom.getAllVolumes()
+self.assertEqual(res, {})
+
+def test_no_templates(self):
+oop = FakeOOP(FakeGlob([
+os.path.join(self.IMAGES_DIR, image-1, volume-1.meta),
+os.path.join(self.IMAGES_DIR, image-1, volume-2.meta),
+os.path.join(self.IMAGES_DIR, image-1, volume-3.meta),
+os.path.join(self.IMAGES_DIR, image-2, volume-4.meta),
+os.path.join(self.IMAGES_DIR, image-2, volume-5.meta),
+os.path.join(self.IMAGES_DIR, image-3, volume-6.meta),
+]))
+dom = TestingFileStorageDomain(self.SD_UUID, self.MOUNTPOINT, oop)
+res = dom.getAllVolumes()
+
+# These volumes should have parent uuid, but the implementation does
+# not read the meta data files, so this info
Change in vdsm[ovirt-3.5]: fileSD: Optimize getAllVolumes on file storage
automat...@ovirt.org has posted comments on this change. Change subject: fileSD: Optimize getAllVolumes on file storage .. Patch Set 1: * Update tracker::#1223053::OK * Check Bug-Url::OK * Check Public Bug::#1223053::OK, public bug * Check Product::#1223053::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::#1223053::OK, correct target release 3.5.2 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/41127 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idc19ce272b7e0b9c91d2f90aa46b5ddd21d69ece Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Adam Litke ali...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
