Change in vdsm[master]: gluster: Temporary fix for supervdsm memory leak.

dnarayan at redhat.com dnarayan at redhat.com
Wed Sep 24 08:27:35 UTC 2014


Darshan N has uploaded a new change for review.

Change subject: gluster: Temporary fix for supervdsm memory leak.
......................................................................

gluster: Temporary fix for supervdsm memory leak.

Supervdsm memory leak is caused by memory leak in libgfapi
which is used by verb glusterVolumeStatsInfoGet to retrive
the volume statistics. This patch provides a temporary fix
until libgfapi issue is resolved.
    In this patch libgfapi call is moved to the script gfapi
which provides volume statistics and this script is executed
using execCmd(), by this libgfapi invocation is not part
of supervdsm process.

Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1142647
Signed-off-by: ndarshan <dnarayan at redhat.com>
---
M Makefile.am
M lib/vdsm/constants.py.in
M vdsm.spec.in
M vdsm/gluster/Makefile.am
M vdsm/gluster/__init__.py
M vdsm/gluster/api.py
M vdsm/gluster/cli.py
R vdsm/gluster/gfapi
8 files changed, 62 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/33312/1

diff --git a/Makefile.am b/Makefile.am
index 88a99c8..cbfbd53 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,6 +70,7 @@
 	contrib/profile-stats \
 	init/daemonAdapter \
 	vdsm/get-conf-item \
+	vdsm/gluster/gfapi \
 	vdsm/set-conf-item \
 	vdsm/supervdsmServer \
 	vdsm/vdsm \
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index ee379d6..daa05f8 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -71,6 +71,7 @@
 #
 # Path definitions
 #
+P_GFAPI_SCRIPT = '@VDSMDIR@/gluster/'
 P_LIBVIRT_VMCHANNELS = '/var/lib/libvirt/qemu/channels/'
 P_VDSM = '@VDSMDIR@/'
 P_VDSM_RPC = '@VDSMDIR@/rpc/'
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 8438df7..28a85e2 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1518,7 +1518,7 @@
 %{_datadir}/%{vdsm_name}/gluster/api.py*
 %{_datadir}/%{vdsm_name}/gluster/apiwrapper.py*
 %{_datadir}/%{vdsm_name}/rpc/vdsmapi-gluster-schema.json
-%{_datadir}/%{vdsm_name}/gluster/gfapi.py*
+%{_datadir}/%{vdsm_name}/gluster/gfapi
 %{_datadir}/%{vdsm_name}/gluster/hooks.py*
 %{_datadir}/%{vdsm_name}/gluster/services.py*
 %{_datadir}/%{vdsm_name}/gluster/tasks.py*
diff --git a/vdsm/gluster/Makefile.am b/vdsm/gluster/Makefile.am
index d1d3ebf..e6e4ca8 100644
--- a/vdsm/gluster/Makefile.am
+++ b/vdsm/gluster/Makefile.am
@@ -32,12 +32,15 @@
 	apiwrapper.py \
 	cli.py \
 	exception.py \
-	gfapi.py \
 	hooks.py \
 	services.py \
 	tasks.py \
 	$(NULL)
 
+dist_vdsmgluster_SCRIPTS = \
+	gfapi \
+	$(NULL)
+
 EXTRA_DIST = \
 	hostname.py.in
 	$(NULL)
diff --git a/vdsm/gluster/__init__.py b/vdsm/gluster/__init__.py
index b83efbb..0aefb8d 100644
--- a/vdsm/gluster/__init__.py
+++ b/vdsm/gluster/__init__.py
@@ -22,7 +22,7 @@
 import tempfile
 from functools import wraps
 
-MODULE_LIST = ('cli', 'hooks', 'services', 'tasks', 'gfapi')
+MODULE_LIST = ('cli', 'hooks', 'services', 'tasks')
 
 
 def makePublic(func):
diff --git a/vdsm/gluster/api.py b/vdsm/gluster/api.py
index b9a08e5..067427b 100644
--- a/vdsm/gluster/api.py
+++ b/vdsm/gluster/api.py
@@ -205,8 +205,8 @@
                                                       replicaCount)
 
     def _computeVolumeStats(self, data):
-        total = data.f_blocks * data.f_bsize
-        free = data.f_bfree * data.f_bsize
+        total = data['f_blocks'] * data['f_bsize']
+        free = data['f_bfree'] * data['f_bsize']
         used = total - free
         return {'sizeTotal': str(total),
                 'sizeFree': str(free),
diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index 2e1c9a9..aab7145 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -19,12 +19,14 @@
 #
 
 import xml.etree.cElementTree as etree
+import json
 
 from vdsm import utils
 from vdsm import netinfo
 import exception as ge
 from hostname import getHostNameFqdn, HostNameException
 from . import makePublic
+from vdsm import constants
 
 _glusterCommandPath = utils.CommandPath("gluster",
                                         "/usr/sbin/gluster",
@@ -1054,3 +1056,20 @@
         return _parseVolumeTasks(xmltree)
     except _etreeExceptions:
         raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
+
+
+# This is a workaround for memory leak caused by the
+# libgfapi used to get volume statistics. Memory accumulates
+# every time the api is invoked to avoid that, it is invoked
+# through the script gfapi.py. This can be reverted back once
+# the memory leak issue is fixed in libgfapi.
+ at makePublic
+def volumeStatvfs(volumeName):
+    script = constants.P_GFAPI_SCRIPT + "gfapi"
+    command = [script, "-v", volumeName]
+    rc, out, err = utils.execCmd(command)
+    if rc != 0:
+        raise ge.GlfsStatvfsException(rc, out, err)
+    json_acceptable_string = out[0].replace("'", "\"")
+    res = json.loads(json_acceptable_string)
+    return res
diff --git a/vdsm/gluster/gfapi.py b/vdsm/gluster/gfapi
similarity index 77%
rename from vdsm/gluster/gfapi.py
rename to vdsm/gluster/gfapi
old mode 100644
new mode 100755
index fc1e4b4..5a5fc3d
--- a/vdsm/gluster/gfapi.py
+++ b/vdsm/gluster/gfapi
@@ -1,4 +1,4 @@
-#
+#!/usr/bin/python
 # Copyright 2014 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -20,9 +20,11 @@
 import ctypes
 from ctypes.util import find_library
 import os
+import sys
+import json
+import argparse
 
 import exception as ge
-from . import makePublic
 
 
 GLUSTER_VOL_PROTOCAL = 'tcp'
@@ -81,7 +83,6 @@
         raise ge.GlfsFiniException(rc=rc)
 
 
- at makePublic
 def volumeStatvfs(volumeId, host=GLUSTER_VOL_HOST,
                   port=GLUSTER_VOL_PORT,
                   protocol=GLUSTER_VOL_PROTOCAL):
@@ -134,3 +135,32 @@
                                  ctypes.c_void_p,
                                  ctypes.c_char_p,
                                  ctypes.c_void_p)(('glfs_statvfs', _lib))
+
+
+# This file is modified to act as a script which can retrive
+# volume statistics using libgfapi. This can be reverted
+# after the memory leak issue is resolved in libgfapi.
+def parse_cmdargs():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-v", "--volume", action="store", type=str,
+                        help="volumeName")
+    parser.add_argument("-ht", "--host", action="store", type=str,
+                        default="localhost", help="host name")
+    parser.add_argument("-p", "--port", action="store", type=int,
+                        default=24007, help="port number")
+    parser.add_argument("-t", "--protocol", action="store", type=str,
+                        default="tcp", help="protocol")
+    args = parser.parse_args()
+    return args
+
+
+if __name__ == '__main__':
+    args = parse_cmdargs()
+    try:
+        res = volumeStatvfs(args.volume, args.host,
+                            args.port, args.protocol)
+    except GlusterLibgfapiException:
+        print "Failed to retrive volume stats"
+        sys.exit(1)
+    print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': res.f_bfree,
+                      'f_bsize': res.f_bsize})


-- 
To view, visit http://gerrit.ovirt.org/33312
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <dnarayan at redhat.com>


More information about the vdsm-patches mailing list