summaryrefslogtreecommitdiffstats
path: root/test/unit
diff options
context:
space:
mode:
authorPrashanth Pai <ppai@redhat.com>2016-03-10 15:30:36 +0530
committerThiago da Silva <thiago@redhat.com>2016-04-04 11:01:36 -0700
commit789f1a150c87f05905b02dfdd8ad77ac6c1ba46f (patch)
tree7589e03ff9d02f3bff90320447582ce4a10d64a3 /test/unit
parentc73037e90c3f551caf18df41efd7fa9750454a10 (diff)
Remove redundant syscalls in POST path
During process of POST requests which updates object metadata (xattrs), the following (ordered) sequence of syscalls were being made twice: open(), fstat(), fgetxattr(), close() Intuitively, one may assume that a getxattr() and setxattr() is enough to fulfil the POST request as it is only supposed to update metadata. But this isn't the case. The above series of syscalls is made first during disk_file.open(). This will trigger an update of all stale metadata (outdated size/etag) and the result is retained in a diskfile class attribute named 'self._metadata' Instead of using this pre-fetched metadata, the POST path was internally invoking disk_file.open() again in disk_file.write_metadata(). This is redundant and serves no purpose. self._metadata was being erased during the context manager cleanup of disk_file.open() This change is simple and does the following: * Don't erase fetched metadata during context manager exit of open() * Use a different internal variable to detect and raise DiskFileNotOpen * Re-use self._metadata if available in disk_file.write_metadata() Here's comparing syscalls made (POST path) with and without this fix: https://bugzilla.redhat.com/show_bug.cgi?id=1314171#c4 Change-Id: Ib64c103e5904428df20ec6e8f10140f4f68e7f79 Signed-off-by: Prashanth Pai <ppai@redhat.com> Reviewed-on: http://review.gluster.org/13668 Reviewed-by: Thiago da Silva <thiago@redhat.com> Tested-by: Thiago da Silva <thiago@redhat.com>
Diffstat (limited to 'test/unit')
-rw-r--r--test/unit/obj/test_diskfile.py12
1 files changed, 12 insertions, 0 deletions
diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py
index 6e2d0b1..9ef186b 100644
--- a/test/unit/obj/test_diskfile.py
+++ b/test/unit/obj/test_diskfile.py
@@ -191,6 +191,7 @@ class TestDiskFile(unittest.TestCase):
gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z")
assert gdf._obj == "z"
assert gdf._fd is None
+ assert gdf._disk_file_open is False
assert gdf._metadata is None
assert not gdf._is_dir
with gdf.open():
@@ -198,6 +199,8 @@ class TestDiskFile(unittest.TestCase):
assert not gdf._is_dir
assert gdf._fd is not None
assert gdf._metadata == exp_md
+ assert gdf._disk_file_open is True
+ assert gdf._disk_file_open is False
self.assertRaises(DiskFileNotOpen, gdf.get_metadata)
self.assertRaises(DiskFileNotOpen, gdf.reader)
self.assertRaises(DiskFileNotOpen, gdf.__enter__)
@@ -233,12 +236,15 @@ class TestDiskFile(unittest.TestCase):
assert gdf._obj == "z"
assert gdf._fd is None
assert gdf._metadata is None
+ assert gdf._disk_file_open is False
assert not gdf._is_dir
with gdf.open():
assert not gdf._is_dir
assert gdf._data_file == the_file
assert gdf._fd is not None
assert gdf._metadata == exp_md, "%r != %r" % (gdf._metadata, exp_md)
+ assert gdf._disk_file_open is True
+ assert gdf._disk_file_open is False
def test_open_invalid_existing_metadata(self):
the_path = os.path.join(self.td, "vol0", "bar")
@@ -256,9 +262,12 @@ class TestDiskFile(unittest.TestCase):
assert gdf._obj == "z"
assert not gdf._is_dir
assert gdf._fd is None
+ assert gdf._disk_file_open is False
with gdf.open():
assert gdf._data_file == the_file
assert gdf._metadata != inv_md
+ assert gdf._disk_file_open is True
+ assert gdf._disk_file_open is False
def test_open_isdir(self):
the_path = os.path.join(self.td, "vol0", "bar")
@@ -278,9 +287,12 @@ class TestDiskFile(unittest.TestCase):
gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "d")
assert gdf._obj == "d"
assert gdf._is_dir is False
+ assert gdf._disk_file_open is False
with gdf.open():
assert gdf._is_dir
assert gdf._data_file == the_dir
+ assert gdf._disk_file_open is True
+ assert gdf._disk_file_open is False
def _create_and_get_diskfile(self, dev, par, acc, con, obj, fsize=256):
# FIXME: assumes account === volume