object-storage: reduce the number of getxattr system calls by one

Initial step towards addressing BZ 865619.

Prior to the patch, reading metadata for a given object or container involves
at minimum three getxattr system calls for objects that use only one xattr
key/value pair:

    1. (via pyxattr)  getxattr() to see if key exists and get its value
       length (so it can allocate memory for second call below)
    2. (from pyxattr) getxattr() to get actual value data
    3. (via pyxattr)  getxattr() to see if following key exists

For objects and containers that only have to use one xattr key/value pair,
this patch reduces the number system calls by one. This can be significant
given that almost every Swift API operation requires reads of the object or
containers metadata.

This patch is mostly a change to plugins.utils.read_metadata() to try to
unpickle the accumulated metadata as each key/value pair is read, rather than
trying to accumulate all the key/value pairs and unpickle at the end. Once we
get enough data to form the pickle, we no longer keep trying to get more keys,
even if those keys exist.

The extra keys can exist when the size of the stored metadata shrinks below a
key threshold. See https://bugzilla.redhat.com/show_bug.cgi?id=865619 for more
details.

See also https://bugzilla.redhat.com/show_bug.cgi?id=865858.

This patch also adds a unit test infrastructure, and uses it to test with full
coverage, the read_metadata, write_metadata and clean_metadata functions. As a
result, we found that an infinite loop would occur in clean_metadata() when an
unexpected IOError would occur trying to remove a key (this patch contains a
fix).

Change-Id: Ia1838c5e73af453b65360c1c525824231aa7c5d4
BUG: 865619
Signed-off-by: Peter Portante <peter.portante@redhat.com>
Reviewed-on: http://review.gluster.org/4109
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Tested-by: Anand Avati <avati@redhat.com>

1 files changed, 43 insertions, 13 deletions

diff --git a/swift/1.4.8/plugins/utils.py b/swift/1.4.8/plugins/utils.py
index 61112c8c..72b2f389 100644
--- a/swift/1.4.8/plugins/utils.py
+++ b/swift/1.4.8/plugins/utils.py
@@ -190,34 +190,63 @@ def _add_timestamp(metadata_i):
 
 def read_metadata(path):
     """
-    Helper function to read the pickled metadata from a File/Directory .
+    Helper function to read the pickled metadata from a File/Directory.
 
     :param path: File/Directory to read metadata from.
 
     :returns: dictionary of metadata
     """
-    metadata = ''
+    metadata = None
+    metadata_s = ''
     key = 0
-    while True:
+    while metadata is None:
         try:
-            metadata += xattr.get(path, '%s%s' % (METADATA_KEY, (key or '')))
+            metadata_s += xattr.get(path, '%s%s' % (METADATA_KEY, (key or '')))
         except IOError as err:
-            if err.errno != errno.ENODATA:
-                logging.exception("xattr.get failed on %s key %s err: %s", path, key, str(err))
-            break
-        key += 1
-    if metadata:
-        return pickle.loads(metadata)
-    else:
-        return {}
+            if err.errno == errno.ENODATA:
+                if key > 0:
+                    # No errors reading the xattr keys, but since we have not
+                    # been able to find enough chunks to get a successful
+                    # unpickle operation, we consider the metadata lost, and
+                    # drop the existing data so that the internal state can be
+                    # recreated.
+                    clean_metadata(path)
+                # We either could not find any metadata key, or we could find
+                # some keys, but were not successful in performing the
+                # unpickling (missing keys perhaps)? Either way, just report
+                # to the caller we have no metadata.
+                metadata = {}
+            else:
+                logging.exception("xattr.get failed on %s key %s err: %s",
+                                  path, key, str(err))
+                # Note that we don't touch the keys on errors fetching the
+                # data since it could be a transient state.
+                raise
+        else:
+            try:
+                # If this key provides all or the remaining part of the pickle
+                # data, we don't need to keep searching for more keys. This
+                # means if we only need to store data in N xattr key/value
+                # pair, we only need to invoke xattr get N times. With large
+                # keys sizes we are shooting for N = 1.
+                metadata = pickle.loads(metadata_s)
+                assert isinstance(metadata, dict)
+            except EOFError, pickle.UnpicklingError:
+                # We still are not able recognize this existing data collected
+                # as a pickled object. Make sure we loop around to try to get
+                # more from another xattr key.
+                metadata = None
+                key += 1
+    return metadata
 
 def write_metadata(path, metadata):
     """
     Helper function to write pickled metadata for a File/Directory.
 
     :param path: File/Directory path to write the metadata
-    :param metadata: metadata to write
+    :param metadata: dictionary to metadata write
     """
+    assert isinstance(metadata, dict)
     metastr = pickle.dumps(metadata, PICKLE_PROTOCOL)
     key = 0
     while metastr:
@@ -237,6 +266,7 @@ def clean_metadata(path):
         except IOError as err:
             if err.errno == errno.ENODATA:
                 break
+            raise
         key += 1
 
 def dir_empty(path):