From 29c23f20a6213affb646c322b7219a0f7c5c3dfc Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Wed, 14 Nov 2012 11:31:43 -0500 Subject: object-storage: use temp file optimization for pkl Override OpenStack Swift's swift.common.utils.write_pickle with our own implementation that uses the GlusterFS temporary file operation. A file name '..' will hash to the same GlusterFS node as a file named '', those avoiding a move of the file on a rename. This is part of the work needed to address BZ 876660 (https://bugzilla.redhat.com/show_bug.cgi?id=876660). Change-Id: I1cb9f97f289ab2ca76ec9221366df74de08268bb BUG: 876660 Signed-off-by: Peter Portante Reviewed-on: http://review.gluster.org/4224 Reviewed-by: Kaleb KEITHLEY Reviewed-by: Mohammed Junaid Tested-by: Gluster Build System --- ufo/gluster/swift/common/utils.py | 31 +++++++++++++++++++++++ ufo/gluster/swift/obj/server.py | 1 + ufo/test/unit/common/test_utils.py | 51 ++++++++++++++++++++++++++++++-------- 3 files changed, 73 insertions(+), 10 deletions(-) (limited to 'ufo') diff --git a/ufo/gluster/swift/common/utils.py b/ufo/gluster/swift/common/utils.py index 56376f8ee..d35abe582 100644 --- a/ufo/gluster/swift/common/utils.py +++ b/ufo/gluster/swift/common/utils.py @@ -17,6 +17,7 @@ import logging import os import errno import xattr +import random from hashlib import md5 import cPickle as pickle from ConfigParser import ConfigParser, NoSectionError, NoOptionError @@ -455,3 +456,33 @@ def create_container_metadata(cont_path, memcache=None): def create_account_metadata(acc_path, memcache=None): metadata = get_account_metadata(acc_path, memcache) return restore_metadata(acc_path, metadata) + +def write_pickle(obj, dest, tmp=None, pickle_protocol=0): + """ + Ensure that a pickle file gets written to disk. The file is first written + to a tmp file location in the destination directory path, ensured it is + synced to disk, then moved to its final destination name. + + This version takes advantage of Gluster's dot-prefix-dot-suffix naming + where the a file named ".thefile.name.9a7aasv" is hashed to the same + Gluster node as "thefile.name". This ensures the renaming of a temp file + once written does not move it to another Gluster node. + + :param obj: python object to be pickled + :param dest: path of final destination file + :param tmp: path to tmp to use, defaults to None (ignored) + :param pickle_protocol: protocol to pickle the obj with, defaults to 0 + """ + dirname = os.path.dirname(dest) + basename = os.path.basename(dest) + tmpname = '.' + basename + '.' + md5(basename + str(random.random())).hexdigest() + tmppath = os.path.join(dirname, tmpname) + with open(tmppath, 'wb') as fo: + pickle.dump(obj, fo, pickle_protocol) + fo.flush() + os.fsync(fo) + os.rename(tmppath, dest) + +# Over-ride Swift's utils.write_pickle with ours +import swift.common.utils +swift.common.utils.write_pickle = write_pickle diff --git a/ufo/gluster/swift/obj/server.py b/ufo/gluster/swift/obj/server.py index 43cdd8890..1c2b6cb1d 100644 --- a/ufo/gluster/swift/obj/server.py +++ b/ufo/gluster/swift/obj/server.py @@ -18,6 +18,7 @@ # Simply importing this monkey patches the constraint handling to fit our # needs import gluster.swift.common.constraints +import gluster.swift.common.utils from swift.obj import server from gluster.swift.common.DiskFile import Gluster_DiskFile diff --git a/ufo/test/unit/common/test_utils.py b/ufo/test/unit/common/test_utils.py index 21acd8e90..446939963 100644 --- a/ufo/test/unit/common/test_utils.py +++ b/ufo/test/unit/common/test_utils.py @@ -417,9 +417,9 @@ class TestUtils(unittest.TestCase): o_count = 3 b_used = 47 return o_list, o_count, b_used - td = tempfile.mkdtemp() orig_gcd = utils.get_container_details utils.get_container_details = _mock_get_container_details + td = tempfile.mkdtemp() try: exp_md = { utils.X_TYPE: (utils.CONTAINER, 0), @@ -439,9 +439,9 @@ class TestUtils(unittest.TestCase): c_list = [ '123', 'abc' ] c_count = 2 return c_list, c_count - td = tempfile.mkdtemp() orig_gad = utils.get_account_details utils.get_account_details = _mock_get_account_details + td = tempfile.mkdtemp() try: exp_md = { utils.X_TYPE: (utils.ACCOUNT, 0), @@ -752,12 +752,13 @@ class TestUtils(unittest.TestCase): utils.do_stat = orig_ds def test_get_container_details_from_fs(self): - td = tempfile.mkdtemp() - tf = tarfile.open("common/data/account_tree.tar.bz2", "r:bz2") orig_cwd = os.getcwd() - os.chdir(td) - tf.extractall() + td = tempfile.mkdtemp() try: + tf = tarfile.open("common/data/account_tree.tar.bz2", "r:bz2") + os.chdir(td) + tf.extractall() + ad = utils._get_account_details_from_fs(td, None) assert ad.mtime == os.path.getmtime(td) assert ad.container_count == 3 @@ -775,12 +776,13 @@ class TestUtils(unittest.TestCase): assert cd.dir_list == [] def test_get_account_details_from_fs(self): - td = tempfile.mkdtemp() - tf = tarfile.open("common/data/container_tree.tar.bz2", "r:bz2") orig_cwd = os.getcwd() - os.chdir(td) - tf.extractall() + td = tempfile.mkdtemp() try: + tf = tarfile.open("common/data/container_tree.tar.bz2", "r:bz2") + os.chdir(td) + tf.extractall() + cd = utils._get_container_details_from_fs(td) assert cd.bytes_used == 30, repr(cd.bytes_used) assert cd.object_count == 8, repr(cd.object_count) @@ -816,3 +818,32 @@ class TestUtils(unittest.TestCase): assert ad.mtime == os.path.getmtime(tf.name) assert ad.container_count == 0 assert ad.container_list == [] + + def test_write_pickle(self): + td = tempfile.mkdtemp() + try: + fpp = os.path.join(td, 'pp') + utils.write_pickle('pickled peppers', fpp) + with open(fpp, "rb") as f: + contents = f.read() + s = pickle.loads(contents) + assert s == 'pickled peppers', repr(s) + finally: + shutil.rmtree(td) + + def test_write_pickle_ignore_tmp(self): + tf = tempfile.NamedTemporaryFile() + td = tempfile.mkdtemp() + try: + fpp = os.path.join(td, 'pp') + # Also test an explicity pickle protocol + utils.write_pickle('pickled peppers', fpp, tmp=tf.name, pickle_protocol=2) + with open(fpp, "rb") as f: + contents = f.read() + s = pickle.loads(contents) + assert s == 'pickled peppers', repr(s) + with open(tf.name, "rb") as f: + contents = f.read() + assert contents == '' + finally: + shutil.rmtree(td) -- cgit