From 9e56bceaccc1d0ac5b167ecc2950aeb4ab319adf Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Tue, 20 May 2014 15:50:55 -0400 Subject: [PATCH 1/5] fix issue with GET on large object Fixing a call to _drop_cache that had an extra parameter A patch to this issue was first provided by Xw Huang(xwhuang@qnap.com) in gluster-swift gerrit. I'm transferring the patch to swiftonfile. Fixes issue #13 Signed-off-by: Thiago da Silva --- gluster/swift/obj/diskfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index 47cde89..3223dcf 100644 --- a/gluster/swift/obj/diskfile.py +++ b/gluster/swift/obj/diskfile.py @@ -509,7 +509,7 @@ class DiskFileReader(object): bytes_read += len(chunk) diff = bytes_read - dropped_cache if diff > (1024 * 1024): - self._drop_cache(self._fd, dropped_cache, diff) + self._drop_cache(dropped_cache, diff) dropped_cache = bytes_read yield chunk if self._iter_hook: From 9eb79c9f499efb025cd21f95cb30528dbafdadea Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Tue, 20 May 2014 19:18:24 -0400 Subject: [PATCH 2/5] adding unit test that would have caught this issue This chunk of code is only executed when reading a large file (> 1014 * 1024), so the issue was never caught in our unit tests refer to issue #13 Signed-off-by: Thiago da Silva --- test/unit/obj/test_diskfile.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index f8c26db..ec813c7 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -285,7 +285,7 @@ class TestDiskFile(unittest.TestCase): assert gdf._data_file == the_dir assert gdf._metadata == exp_md - def _create_and_get_diskfile(self, dev, par, acc, con, obj): + def _create_and_get_diskfile(self, dev, par, acc, con, obj, fsize=256): # FIXME: assumes account === volume the_path = os.path.join(self.td, dev, con) the_file = os.path.join(the_path, obj) @@ -293,7 +293,7 @@ class TestDiskFile(unittest.TestCase): base_dir = os.path.dirname(the_file) os.makedirs(base_dir) with open(the_file, "wb") as fd: - fd.write("y" * 256) + fd.write("y" * fsize) gdf = self._get_diskfile(dev, par, acc, con, obj) assert gdf._obj == base_obj assert not gdf._is_dir @@ -353,6 +353,26 @@ class TestDiskFile(unittest.TestCase): assert len(chunks) == 1, repr(chunks) assert called[0] == 1, called + def test_reader_larger_file(self): + closed = [False] + fd = [-1] + + def mock_close(*args, **kwargs): + closed[0] = True + os.close(fd[0]) + + with mock.patch("gluster.swift.obj.diskfile.do_close", mock_close): + gdf = self._create_and_get_diskfile("vol0", "p57", "ufo47", "bar", "z", fsize=1024*1024*2) + with gdf.open(): + assert gdf._fd is not None + assert gdf._data_file == os.path.join(self.td, "vol0", "bar", "z") + reader = gdf.reader() + assert reader._fd is not None + fd[0] = reader._fd + chunks = [ck for ck in reader] + assert reader._fd is None + assert closed[0] + def test_reader_dir_object(self): called = [False] From 006dfba79e93235a1efb87b3fe7839e4693bf6f7 Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Wed, 21 May 2014 11:30:44 -0400 Subject: [PATCH 3/5] adding functional test to cover same issue functional test writes and read a large object to exercise code that calls drop_cache after reading 1MB of data Signed-off-by: Thiago da Silva --- test/functional/gluster_swift_tests.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/functional/gluster_swift_tests.py b/test/functional/gluster_swift_tests.py index 2768f9d..d34a082 100644 --- a/test/functional/gluster_swift_tests.py +++ b/test/functional/gluster_swift_tests.py @@ -16,6 +16,7 @@ """ OpenStack Swift based functional tests for Gluster for Swift""" import random +import time import os,sys,re,hashlib from nose import SkipTest @@ -58,6 +59,14 @@ class TestFile(Base): data_read = file.read() self.assertEquals(data,data_read) + def test_PUT_large_object(self): + file_item = self.env.container.file(Utils.create_name()) + data = File.random_data(1024 * 1024 * 2) + self.assertTrue(file_item.write(data)) + self.assert_status(201) + self.assertTrue(data == file_item.read()) + self.assert_status(200) + def testInvalidHeadersPUT(self): #TODO: Although we now support x-delete-at and x-delete-after, #retained this test case as we may add some other header to From ed49689509decc42656e555b5b2ce59dc9291d2d Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Wed, 21 May 2014 11:30:44 -0400 Subject: [PATCH 4/5] adding functional test to cover same issue functional test writes and read a large object to exercise code that calls drop_cache after reading 1MB of data Signed-off-by: Thiago da Silva --- test/functional/gluster_swift_tests.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/functional/gluster_swift_tests.py b/test/functional/gluster_swift_tests.py index 2768f9d..7ba27de 100644 --- a/test/functional/gluster_swift_tests.py +++ b/test/functional/gluster_swift_tests.py @@ -58,6 +58,14 @@ class TestFile(Base): data_read = file.read() self.assertEquals(data,data_read) + def test_PUT_large_object(self): + file_item = self.env.container.file(Utils.create_name()) + data = File.random_data(1024 * 1024 * 2) + self.assertTrue(file_item.write(data)) + self.assert_status(201) + self.assertTrue(data == file_item.read()) + self.assert_status(200) + def testInvalidHeadersPUT(self): #TODO: Although we now support x-delete-at and x-delete-after, #retained this test case as we may add some other header to From 49cddb0f1eadd6cc6adfac24976a751c1d260bb2 Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Wed, 21 May 2014 11:37:20 -0400 Subject: [PATCH 5/5] removing unused time module Signed-off-by: Thiago da Silva --- test/functional/gluster_swift_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/functional/gluster_swift_tests.py b/test/functional/gluster_swift_tests.py index d34a082..7ba27de 100644 --- a/test/functional/gluster_swift_tests.py +++ b/test/functional/gluster_swift_tests.py @@ -16,7 +16,6 @@ """ OpenStack Swift based functional tests for Gluster for Swift""" import random -import time import os,sys,re,hashlib from nose import SkipTest