From 1338ad168a4a468636909322dace9dc9f750dd13 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Mon, 10 Dec 2012 00:41:27 -0500 Subject: object-storage: apply upstream DiskFile refactor Apply the upstream DiskFile refactoring in ahead of its use to easiliy apply the temp file optimization. Change-Id: I2708733eed3d87759c70eb3d9e6cd74ef91d0c7b BUG: 876660 Signed-off-by: Peter Portante Reviewed-on: http://review.gluster.org/4288 Tested-by: Gluster Build System Reviewed-by: Mohammed Junaid Reviewed-by: Vijay Bellur --- ufo/gluster/swift/common/DiskFile.py | 89 ++++++++++++++++++----------------- ufo/test/unit/common/test_diskfile.py | 65 +++++++++++++------------ 2 files changed, 81 insertions(+), 73 deletions(-) (limited to 'ufo') diff --git a/ufo/gluster/swift/common/DiskFile.py b/ufo/gluster/swift/common/DiskFile.py index 26abbbef..5698ff72 100644 --- a/ufo/gluster/swift/common/DiskFile.py +++ b/ufo/gluster/swift/common/DiskFile.py @@ -41,6 +41,26 @@ class AlreadyExistsAsDir(Exception): pass +def _adjust_metadata(metadata): + # Fix up the metadata to ensure it has a proper value for the + # Content-Type metadata, as well as an X_TYPE and X_OBJECT_TYPE + # metadata values. + content_type = metadata['Content-Type'] + if not content_type: + # FIXME: How can this be that our caller supplied us with metadata + # that has a content type that evaluates to False? + # + # FIXME: If the file exists, we would already know it is a + # directory. So why are we assuming it is a file object? + metadata['Content-Type'] = FILE_TYPE + x_object_type = FILE + else: + x_object_type = MARKER_DIR if content_type.lower() == DIR_TYPE else FILE + metadata[X_TYPE] = OBJECT + metadata[X_OBJECT_TYPE] = x_object_type + return metadata + + class Gluster_DiskFile(DiskFile): """ Manage object files on disk. @@ -83,6 +103,7 @@ class Gluster_DiskFile(DiskFile): self._container_path = os.path.join(path, device, container) self._is_dir = False self.tmpdir = os.path.join(path, device, 'tmp') + self.tmppath = None self.logger = logger self.metadata = {} self.meta_file = None @@ -143,7 +164,7 @@ class Gluster_DiskFile(DiskFile): """ return not self.data_file - def create_dir_object(self, dir_path): + def _create_dir_object(self, dir_path): #TODO: if object already exists??? if os.path.exists(dir_path) and not os.path.isdir(dir_path): self.logger.error("Deleting file %s", dir_path) @@ -153,60 +174,42 @@ class Gluster_DiskFile(DiskFile): do_chown(dir_path, self.uid, self.gid) create_object_metadata(dir_path) - def put_metadata(self, metadata): + def put_metadata(self, metadata, tombstone=False): + """ + Short hand for putting metadata to .meta and .ts files. + + :param metadata: dictionary of metadata to be written + :param tombstone: whether or not we are writing a tombstone + """ + if tombstone: + # We don't write tombstone files. So do nothing. + return assert self.data_file is not None, "put_metadata: no file to put metadata into" + metadata = _adjust_metadata(metadata) write_metadata(self.data_file, metadata) self.metadata = metadata + self.filter_metadata() - def put(self, fd, tmppath, metadata, extension=''): + def put(self, fd, metadata, extension='.data'): """ Finalize writing the file on disk, and renames it from the temp file to the real location. This should be called after the data has been written to the temp file. - :params fd: file descriptor of the temp file - :param tmppath: path to the temporary file being used + :param fd: file descriptor of the temp file :param metadata: dictionary of metadata to be written - :param extention: extension to be used when making the file + :param extension: extension to be used when making the file """ - if extension == '.ts': - # TombStone marker (deleted) - return - - # Fix up the metadata to ensure it has a proper value for the - # Content-Type metadata, as well as an X_TYPE and X_OBJECT_TYPE - # metadata values. - - content_type = metadata['Content-Type'] - if not content_type: - # FIXME: How can this be that our caller supplied us with metadata - # that has a content type that evaluates to False? - # - # FIXME: If the file exists, we would already know it is a - # directory. So why are we assuming it is a file object? - metadata['Content-Type'] = FILE_TYPE - x_object_type = FILE - else: - x_object_type = MARKER_DIR if content_type.lower() == DIR_TYPE else FILE - metadata[X_TYPE] = OBJECT - metadata[X_OBJECT_TYPE] = x_object_type - - if extension == '.meta': - # Metadata recorded separately from the file, we just update the - # metadata for the file. - # - # FIXME: If the file does not exist, this call will fail. - self.put_metadata(metadata) - return - # Our caller will use '.data' here; we just ignore it since we map the # URL directly to the file system. extension = '' + metadata = _adjust_metadata(metadata) + if metadata[X_OBJECT_TYPE] == MARKER_DIR: if not self.data_file: self.data_file = os.path.join(self.datadir, self._obj) - self.create_dir_object(self.data_file) + self._create_dir_object(self.data_file) self.put_metadata(metadata) return @@ -218,7 +221,7 @@ class Gluster_DiskFile(DiskFile): raise AlreadyExistsAsDir(msg) timestamp = normalize_timestamp(metadata[X_TIMESTAMP]) - write_metadata(tmppath, metadata) + write_metadata(self.tmppath, metadata) if X_CONTENT_LENGTH in metadata: self.drop_cache(fd, 0, int(metadata[X_CONTENT_LENGTH])) tpool.execute(os.fsync, fd) @@ -228,13 +231,14 @@ class Gluster_DiskFile(DiskFile): tmp_path = self._container_path for dir_name in dir_objs: tmp_path = os.path.join(tmp_path, dir_name) - self.create_dir_object(tmp_path) + self._create_dir_object(tmp_path) newpath = os.path.join(self.datadir, self._obj) - renamer(tmppath, newpath) + renamer(self.tmppath, newpath) do_chown(newpath, self.uid, self.gid) self.metadata = metadata self.data_file = newpath + self.filter_metadata() return def unlinkold(self, timestamp): @@ -302,14 +306,15 @@ class Gluster_DiskFile(DiskFile): if not os.path.exists(self.tmpdir): mkdirs(self.tmpdir) - fd, tmppath = mkstemp(dir=self.tmpdir) + fd, self.tmppath = mkstemp(dir=self.tmpdir) try: - yield fd, tmppath + yield fd finally: try: os.close(fd) except OSError: pass + tmppath, self.tmppath = self.tmppath, None try: os.unlink(tmppath) except OSError: diff --git a/ufo/test/unit/common/test_diskfile.py b/ufo/test/unit/common/test_diskfile.py index 264c656a..7583fd7f 100644 --- a/ufo/test/unit/common/test_diskfile.py +++ b/ufo/test/unit/common/test_diskfile.py @@ -315,7 +315,7 @@ class TestDiskFile(unittest.TestCase): "dir/z", self.lg) # Not created, dir object path is different, just checking assert gdf._obj == "z" - gdf.create_dir_object(the_dir) + gdf._create_dir_object(the_dir) assert os.path.isdir(the_dir) assert the_dir in _metadata finally: @@ -339,7 +339,7 @@ class TestDiskFile(unittest.TestCase): dc = gluster.swift.common.DiskFile.do_chown gluster.swift.common.DiskFile.do_chown = _mock_do_chown try: - gdf.create_dir_object(the_dir) + gdf._create_dir_object(the_dir) finally: gluster.swift.common.DiskFile.do_chown = dc assert os.path.isdir(the_dir) @@ -355,9 +355,9 @@ class TestDiskFile(unittest.TestCase): os.makedirs(the_dir) gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "z", self.lg) - md = { 'a': 'b' } - gdf.put_metadata(md) - assert gdf.metadata == md + md = { 'Content-Type': 'application/octet-stream', 'a': 'b' } + gdf.put_metadata(md.copy()) + assert gdf.metadata == md, "gdf.metadata = %r, md = %r" % (gdf.metadata, md) assert _metadata[the_dir] == md finally: shutil.rmtree(td) @@ -368,7 +368,7 @@ class TestDiskFile(unittest.TestCase): "z", self.lg) assert gdf.metadata == {} - gdf.put(None, '', {'x': '1'}, extension='.ts') + gdf.put_metadata({'x': '1'}, tombstone=True) assert gdf.metadata == {} def test_put_w_meta_file(self): @@ -383,8 +383,7 @@ class TestDiskFile(unittest.TestCase): "z", self.lg) newmd = gdf.metadata.copy() newmd['X-Object-Meta-test'] = '1234' - with gdf.mkstemp() as (fd, tmppath): - gdf.put(fd, tmppath, newmd, extension='.meta') + gdf.put_metadata(newmd) assert gdf.metadata == newmd assert _metadata[the_file] == newmd finally: @@ -403,8 +402,7 @@ class TestDiskFile(unittest.TestCase): newmd = gdf.metadata.copy() newmd['Content-Type'] = '' newmd['X-Object-Meta-test'] = '1234' - with gdf.mkstemp() as (fd, tmppath): - gdf.put(fd, tmppath, newmd, extension='.meta') + gdf.put_metadata(newmd) assert gdf.metadata == newmd assert _metadata[the_file] == newmd finally: @@ -420,7 +418,7 @@ class TestDiskFile(unittest.TestCase): "dir", self.lg) newmd = gdf.metadata.copy() newmd['X-Object-Meta-test'] = '1234' - gdf.put(None, None, newmd, extension='.meta') + gdf.put_metadata(newmd) assert gdf.metadata == newmd assert _metadata[the_dir] == newmd finally: @@ -436,7 +434,7 @@ class TestDiskFile(unittest.TestCase): "dir", self.lg) newmd = gdf.metadata.copy() newmd['X-Object-Meta-test'] = '1234' - gdf.put(None, None, newmd, extension='.data') + gdf.put_metadata(newmd) assert gdf.metadata == newmd assert _metadata[the_dir] == newmd finally: @@ -455,7 +453,8 @@ class TestDiskFile(unittest.TestCase): 'ETag': 'etag', 'X-Timestamp': 'ts', 'Content-Type': 'application/directory'} - gdf.put(None, None, newmd) + gdf.put(None, newmd, extension='.dir') + assert gdf.data_file == the_dir assert gdf.metadata == newmd assert _metadata[the_dir] == newmd finally: @@ -477,7 +476,7 @@ class TestDiskFile(unittest.TestCase): newmd['Content-Type'] = '' newmd['X-Object-Meta-test'] = '1234' try: - gdf.put(None, None, newmd, extension='.data') + gdf.put(None, newmd, extension='.data') except AlreadyExistsAsDir: pass else: @@ -509,9 +508,11 @@ class TestDiskFile(unittest.TestCase): 'Content-Length': '5', } - with gdf.mkstemp() as (fd, tmppath): + with gdf.mkstemp() as fd: + assert gdf.tmppath is not None + tmppath = gdf.tmppath os.write(fd, body) - gdf.put(fd, tmppath, metadata) + gdf.put(fd, metadata) assert gdf.data_file == os.path.join(td, "vol0", "bar", "z") assert os.path.exists(gdf.data_file) @@ -543,9 +544,11 @@ class TestDiskFile(unittest.TestCase): 'Content-Length': '5', } - with gdf.mkstemp() as (fd, tmppath): + with gdf.mkstemp() as fd: + assert gdf.tmppath is not None + tmppath = gdf.tmppath os.write(fd, body) - gdf.put(fd, tmppath, metadata) + gdf.put(fd, metadata) assert gdf.data_file == os.path.join(td, "vol0", "bar", "b", "a", "z") assert os.path.exists(gdf.data_file) @@ -844,12 +847,12 @@ class TestDiskFile(unittest.TestCase): gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "dir/z", self.lg) saved_tmppath = '' - with gdf.mkstemp() as (fd, tmppath): + with gdf.mkstemp() as fd: assert gdf.tmpdir == os.path.join(td, "vol0", "tmp") assert os.path.isdir(gdf.tmpdir) - assert os.path.dirname(tmppath) == gdf.tmpdir - assert os.path.exists(tmppath) - saved_tmppath = tmppath + saved_tmppath = gdf.tmppath + assert os.path.dirname(saved_tmppath) == gdf.tmpdir + assert os.path.exists(saved_tmppath) os.write(fd, "123") assert not os.path.exists(saved_tmppath) finally: @@ -863,12 +866,12 @@ class TestDiskFile(unittest.TestCase): gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "dir/z", self.lg) saved_tmppath = '' - with gdf.mkstemp() as (fd, tmppath): + with gdf.mkstemp() as fd: assert gdf.tmpdir == os.path.join(td, "vol0", "tmp") assert os.path.isdir(gdf.tmpdir) - assert os.path.dirname(tmppath) == gdf.tmpdir - assert os.path.exists(tmppath) - saved_tmppath = tmppath + saved_tmppath = gdf.tmppath + assert os.path.dirname(saved_tmppath) == gdf.tmpdir + assert os.path.exists(saved_tmppath) os.write(fd, "123") os.close(fd) assert not os.path.exists(saved_tmppath) @@ -883,14 +886,14 @@ class TestDiskFile(unittest.TestCase): gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar", "dir/z", self.lg) saved_tmppath = '' - with gdf.mkstemp() as (fd, tmppath): + with gdf.mkstemp() as fd: assert gdf.tmpdir == os.path.join(td, "vol0", "tmp") assert os.path.isdir(gdf.tmpdir) - assert os.path.dirname(tmppath) == gdf.tmpdir - assert os.path.exists(tmppath) - saved_tmppath = tmppath + saved_tmppath = gdf.tmppath + assert os.path.dirname(saved_tmppath) == gdf.tmpdir + assert os.path.exists(saved_tmppath) os.write(fd, "123") - os.unlink(tmppath) + os.unlink(saved_tmppath) assert not os.path.exists(saved_tmppath) finally: shutil.rmtree(td) -- cgit