Fix bug 251037: make packing blob storages non-blocking

NEWS.txt

@@ -5,6 +5,8 @@ Whats new in ZODB 3.8.1
 
 Bugs Fixed:
 
+- (???) Fixed bug #251037: Made packing of blob storages non-blocking.
+
 - (beta 6) Fixed a bug that could cause InvalidObjectReference errors
   for objects that were explicitly added to a database if the object
   was modified after a savepoint that added the object.

src/ZODB/blob.py

@@ -392,6 +392,10 @@ class FilesystemHelper:
             yield oid, self.getPathForOID(oid)
 
 
+class BlobStorageError(Exception):
+    """The blob storage encountered an invalid state."""
+
+
 class BlobStorage(SpecificationDecoratorBase):
     """A storage to support blobs."""
 
@@ -399,7 +403,8 @@ class BlobStorage(SpecificationDecoratorBase):
 
     # Proxies can't have a __dict__ so specifying __slots__ here allows
     # us to have instance attributes explicitly on the proxy.
-    __slots__ = ('fshelper', 'dirty_oids', '_BlobStorage__supportsUndo')
+    __slots__ = ('fshelper', 'dirty_oids', '_BlobStorage__supportsUndo',
+                 '_blobs_pack_is_in_progress', )
 
     def __new__(self, base_directory, storage):
         return SpecificationDecoratorBase.__new__(self, storage)
@@ -418,6 +423,7 @@ class BlobStorage(SpecificationDecoratorBase):
         else:
             supportsUndo = supportsUndo()
         self.__supportsUndo = supportsUndo
+        self._blobs_pack_is_in_progress = False
 
     @non_overridable
     def temporaryDirectory(self):
@@ -529,21 +535,29 @@ class BlobStorage(SpecificationDecoratorBase):
 
     @non_overridable
     def pack(self, packtime, referencesf):
-        """Remove all unused oid/tid combinations."""
-        unproxied = getProxiedObject(self)
-
-        # pack the underlying storage, which will allow us to determine
-        # which serials are current.
-        result = unproxied.pack(packtime, referencesf)
-
-        # perform a pack on blob data
+        """Remove all unused OID/TID combinations."""
         self._lock_acquire()
         try:
+            if self._blobs_pack_is_in_progress:
+                raise BlobStorageError('Already packing')
+            self._blobs_pack_is_in_progress = True
+        finally:
+            self._lock_release()
+
+        try:
+            # Pack the underlying storage, which will allow us to determine
+            # which serials are current.
+            unproxied = getProxiedObject(self)
+            result = unproxied.pack(packtime, referencesf)
+
+            # Perform a pack on the blob data.
             if self.__supportsUndo:
                 self._packUndoing(packtime, referencesf)
             else:
                 self._packNonUndoing(packtime, referencesf)
         finally:
+            self._lock_acquire()
+            self._blobs_pack_is_in_progress = False
             self._lock_release()
 
         return result

src/ZODB/tests/blob_packing.txt

@@ -240,6 +240,37 @@ revision as well as the entire directory:
     >>> os.path.exists(os.path.split(fns[0])[0])
     False
 
+Avoiding parallel packs
+=======================
+
+Blob packing (similar to FileStorage) can only be run once at a time. For
+this, a flag (_blobs_pack_is_in_progress) is set. If the pack method is called
+while this flag is set, it will refuse to perform another pack, until the flag
+is reset:
+
+    >>> blob_storage._blobs_pack_is_in_progress
+    False
+    >>> blob_storage._blobs_pack_is_in_progress = True
+    >>> blob_storage.pack(packtime, referencesf)
+    Traceback (most recent call last):
+    BlobStorageError: Already packing
+    >>> blob_storage._blobs_pack_is_in_progress = False
+    >>> blob_storage.pack(packtime, referencesf)
+
+We can also see, that the flag is set during the pack, by leveraging the
+knowledge that the underlying storage's pack method is also called:
+
+    >>> def dummy_pack(time, ref):
+    ...     print "_blobs_pack_is_in_progress =", blob_storage._blobs_pack_is_in_progress
+    ...     return base_pack(time, ref)
+    >>> base_pack = base_storage.pack
+    >>> base_storage.pack = dummy_pack
+    >>> blob_storage.pack(packtime, referencesf)
+    _blobs_pack_is_in_progress = True
+    >>> blob_storage._blobs_pack_is_in_progress
+    False
+    >>> base_storage.pack = base_pack
+
 Clean up our blob directory:
 
     >>> shutil.rmtree(blob_dir)