From bd62696fa9fe694a1053e6c23f8c7ee0c66b76b6 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Tue, 20 Aug 2013 18:32:28 -0400 Subject: Fix infinite loop for temp file renames on ENOENT This is a port from master branch fix http://review.gluster.org/5670 to grizzly branch. For whatever reason, it appears that GlusterFS, or perhaps FUSE can continuously return ENOENT on a rename system call even when we have double checked that there is no reason to do so. That is a bug for that sub system. However, our response to that bug can result in an infinite loop, which is bad. This code reduces that to 10 attempts. In addition, we restructed the open retry loop to match, providing module constants for the upper bounds of both retry loops. BUG: 1005379 (https://bugzilla.redhat.com/show_bug.cgi?id=1005379) Change-Id: Ia2d6dd427daba3ea0461863c5ffe3aef27c88f9b Signed-off-by: Peter Portante Reviewed-on: http://review.gluster.org/5670 Reviewed-by: Luis Pabon Tested-by: Luis Pabon Signed-off-by: Luis Pabon Reviewed-on: http://review.gluster.org/5848 Reviewed-by: Peter Portante Tested-by: Peter Portante --- test/unit/common/test_diskfile.py | 195 +++++++++++++++++++++++++++++++++++++- test/unit/common/test_utils.py | 5 +- 2 files changed, 196 insertions(+), 4 deletions(-) (limited to 'test/unit') diff --git a/test/unit/common/test_diskfile.py b/test/unit/common/test_diskfile.py index 410f113..564978f 100644 --- a/test/unit/common/test_diskfile.py +++ b/test/unit/common/test_diskfile.py @@ -25,11 +25,12 @@ import mock from mock import patch from hashlib import md5 -import gluster.swift.common.utils -import gluster.swift.common.DiskFile from swift.common.utils import normalize_timestamp -from gluster.swift.common.DiskFile import Gluster_DiskFile from swift.common.exceptions import DiskFileNotExist, DiskFileError + +from gluster.swift.common.DiskFile import Gluster_DiskFile, GlusterFileSystemOSError +import gluster.swift.common.utils +import gluster.swift.common.DiskFile from gluster.swift.common.utils import DEFAULT_UID, DEFAULT_GID, X_TYPE, \ X_OBJECT_TYPE, DIR_OBJECT from test_utils import _initxattr, _destroyxattr @@ -596,6 +597,7 @@ class TestDiskFile(unittest.TestCase): 'ETag': etag, 'Content-Length': '5', } + def mock_open(*args, **kwargs): raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC)) @@ -611,6 +613,193 @@ class TestDiskFile(unittest.TestCase): finally: shutil.rmtree(td) + def test_put_rename_ENOENT(self): + td = tempfile.mkdtemp() + the_cont = os.path.join(td, "vol0", "bar") + try: + os.makedirs(the_cont) + gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "z", self.lg) + assert gdf._obj == "z" + assert gdf._obj_path == "" + assert gdf.name == "bar" + assert gdf.datadir == the_cont + assert gdf.data_file is None + + body = '1234\n' + etag = md5() + etag.update(body) + etag = etag.hexdigest() + metadata = { + 'X-Timestamp': '1234', + 'Content-Type': 'file', + 'ETag': etag, + 'Content-Length': '5', + } + + def mock_sleep(*args, **kwargs): + # Return without sleep, no need to dely unit tests + return + + def mock_rename(*args, **kwargs): + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT)) + + with mock.patch("gluster.swift.common.DiskFile.sleep", mock_sleep): + with mock.patch("os.rename", mock_rename): + try: + with gdf.mkstemp() as fd: + assert gdf.tmppath is not None + tmppath = gdf.tmppath + os.write(fd, body) + gdf.put(fd, metadata) + except GlusterFileSystemOSError: + pass + else: + self.fail("Expected exception DiskFileError") + finally: + shutil.rmtree(td) + + + def test_put_rename_ENOENT_filename_conflict(self): + td = tempfile.mkdtemp() + the_cont = os.path.join(td, "vol0", "bar") + try: + os.makedirs(the_cont) + gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "z", self.lg) + self.assertEqual(gdf._obj, "z") + self.assertEqual(gdf._obj_path, "") + self.assertEqual(gdf.name, "bar") + self.assertEqual(gdf.datadir, the_cont) + self.assertEqual(gdf.data_file, None) + + body = '1234\n' + etag = md5() + etag.update(body) + etag = etag.hexdigest() + metadata = { + 'X-Timestamp': '1234', + 'Content-Type': 'file', + 'ETag': etag, + 'Content-Length': '5', + } + class MockOSStat: + pass + + def mock_sleep(*args, **kwargs): + # Return without sleep, no need to dely unit tests + return + + def mock_rename(*args, **kwargs): + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT)) + + def mock_do_stat(*args, **kwars): + stat = MockOSStat() + stat.st_ino = 1 + return stat + + def mock_do_fstat(*args, **kwars): + stat = MockOSStat() + stat.st_ino = 2 + return stat + + with mock.patch("gluster.swift.common.DiskFile.sleep", mock_sleep): + with mock.patch("os.rename", mock_rename): + with mock.patch("gluster.swift.common.DiskFile.do_stat", mock_do_stat): + with mock.patch("gluster.swift.common.DiskFile.do_fstat", mock_do_fstat): + with gdf.mkstemp() as fd: + self.assertNotEqual(gdf.tmppath, None) + os.write(fd, body) + self.assertRaises(DiskFileError, gdf.put, fd, metadata) + finally: + shutil.rmtree(td) + + def test_put_rename_ENOENT_bad_path_datafile_target(self): + td = tempfile.mkdtemp() + the_cont = os.path.join(td, "vol0", "bar") + try: + os.makedirs(the_cont) + gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "z", self.lg) + self.assertEqual(gdf._obj, "z") + self.assertEqual(gdf._obj_path, "") + self.assertEqual(gdf.name, "bar") + self.assertEqual(gdf.datadir, the_cont) + self.assertEqual(gdf.data_file, None) + + body = '1234\n' + etag = md5() + etag.update(body) + etag = etag.hexdigest() + metadata = { + 'X-Timestamp': '1234', + 'Content-Type': 'file', + 'ETag': etag, + 'Content-Length': '5', + } + + def mock_sleep(*args, **kwargs): + # Return without sleep, no need to dely unit tests + return + + def mock_rename(*args, **kwargs): + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT)) + + with mock.patch("gluster.swift.common.DiskFile.sleep", mock_sleep): + with mock.patch("os.rename", mock_rename): + with gdf.mkstemp() as fd: + self.assertNotEqual(gdf.tmppath, None) + os.write(fd, body) + + # Purpusely make the datadir non-existent + nonexistdir = tempfile.mkdtemp() + shutil.rmtree(nonexistdir) + gdf.put_datadir = nonexistdir + + self.assertRaises(DiskFileError, gdf.put, fd, metadata) + finally: + shutil.rmtree(td) + + def test_put_rename_ENOENT_target_no_longer_a_dir(self): + td = tempfile.mkdtemp() + the_cont = os.path.join(td, "vol0", "bar") + try: + os.makedirs(the_cont) + gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "z", self.lg) + self.assertEqual(gdf._obj, "z") + self.assertEqual(gdf._obj_path, "") + self.assertEqual(gdf.name, "bar") + self.assertEqual(gdf.datadir, the_cont) + self.assertEqual(gdf.data_file, None) + + body = '1234\n' + etag = md5() + etag.update(body) + etag = etag.hexdigest() + metadata = { + 'X-Timestamp': '1234', + 'Content-Type': 'file', + 'ETag': etag, + 'Content-Length': '5', + } + + def mock_sleep(*args, **kwargs): + # Return without sleep, no need to dely unit tests + return + + def mock_rename(*args, **kwargs): + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT)) + + def mock_stat_S_ISDIR(*args, **kwargs): + return False + + with mock.patch("gluster.swift.common.DiskFile.sleep", mock_sleep): + with mock.patch("os.rename", mock_rename): + with mock.patch("gluster.swift.common.DiskFile.stat.S_ISDIR", mock_stat_S_ISDIR): + with gdf.mkstemp() as fd: + self.assertNotEqual(gdf.tmppath, None) + os.write(fd, body) + self.assertRaises(DiskFileError, gdf.put, fd, metadata) + finally: + shutil.rmtree(td) + def test_put_obj_path(self): the_obj_path = os.path.join("b", "a") the_file = os.path.join(the_obj_path, "z") diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 4aae4c3..e86a0e9 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -585,10 +585,12 @@ class TestUtils(unittest.TestCase): os.chdir(orig_cwd) shutil.rmtree(td) - def test_get_container_details(self): + def test_get_container_details_and_size(self): orig_cwd = os.getcwd() + __do_getsize = Glusterfs._do_getsize td = tempfile.mkdtemp() try: + Glusterfs._do_getsize = False tf = tarfile.open("common/data/container_tree.tar.bz2", "r:bz2") os.chdir(td) tf.extractall() @@ -610,6 +612,7 @@ class TestUtils(unittest.TestCase): full_dir3: os.path.getmtime(full_dir3), } finally: + Glusterfs._do_getsize = __do_getsize os.chdir(orig_cwd) shutil.rmtree(td) -- cgit