Make DirectoryStore __setitem__ resilient against antivirus file locking (#698)

* Update storage.py

* Resolve F841 warning, add comments

* pep8 line length issue

* resolve other f841 warning

* Trigger new build, linux testing failed with http error

* apply suggested changes from Josh Moore, provides test for new retry_call

* add optional argument typehint

* Resolve typehint issues

* bug with typehint

* unused import

* bug with type hint

* Simplify and add code coverage tests

* Add PermissionError exception to retry_call

Co-authored-by: jmoore <[email protected]>
Co-authored-by: Josh Moore <[email protected]>

Author: ericgyounkin

zarr/storage.py

@@ -53,7 +53,7 @@
 from zarr.meta import encode_array_metadata, encode_group_metadata
 from zarr.util import (buffer_size, json_loads, nolock, normalize_chunks,
                        normalize_dtype, normalize_fill_value, normalize_order,
-                       normalize_shape, normalize_storage_path)
+                       normalize_shape, normalize_storage_path, retry_call)
 
 __doctest_requires__ = {
     ('RedisStore', 'RedisStore.*'): ['redis'],
@@ -860,8 +860,10 @@ def __setitem__(self, key, value):
         try:
             self._tofile(value, temp_path)
 
-            # move temporary file into place
-            os.replace(temp_path, file_path)
+            # move temporary file into place;
+            # make several attempts at writing the temporary file to get past
+            # potential antivirus file locking issues
+            retry_call(os.replace, (temp_path, file_path), exceptions=(PermissionError,))
 
         finally:
             # clean up if temp file still exists for whatever reason

zarr/tests/test_util.py

@@ -7,7 +7,7 @@
 from zarr.util import (guess_chunks, human_readable_size, info_html_report,
                        info_text_report, is_total_slice, normalize_chunks,
                        normalize_fill_value, normalize_order,
-                       normalize_resize_args, normalize_shape,
+                       normalize_resize_args, normalize_shape, retry_call,
                        tree_array_icon, tree_group_icon, tree_get_icon,
                        tree_widget)
 
@@ -175,3 +175,30 @@ def test_tree_widget_missing_ipytree():
         )
     with pytest.raises(ImportError, match=re.escape(pattern)):
         tree_widget(None, None, None)
+
+
+def test_retry_call():
+
+    class Fixture:
+
+        def __init__(self, pass_on=1):
+            self.c = 0
+            self.pass_on = pass_on
+
+        def __call__(self):
+            self.c += 1
+            if self.c != self.pass_on:
+                raise PermissionError()
+
+    for x in range(1, 11):
+        # Any number of failures less than 10 will be accepted.
+        fixture = Fixture(pass_on=x)
+        retry_call(fixture, exceptions=(PermissionError,), wait=0)
+        assert fixture.c == x
+
+    def fail(x):
+        # Failures after 10 will cause an error to be raised.
+        retry_call(Fixture(pass_on=x), exceptions=(Exception,), wait=0)
+
+    for x in range(11, 15):
+        pytest.raises(PermissionError, fail, x)

zarr/util.py

@@ -4,6 +4,7 @@
 import numbers
 from textwrap import TextWrapper
 import mmap
+import time
 
 import numpy as np
 from asciitree import BoxStyle, LeftAligned
@@ -12,7 +13,8 @@
 from numcodecs.registry import codec_registry
 from numcodecs.blosc import cbuffer_sizes, cbuffer_metainfo
 
-from typing import Any, Dict, Tuple, Union
+from typing import Any, Callable, Dict, Tuple, Union
+
 
 # codecs to use for object dtype convenience API
 object_codecs = {
@@ -609,3 +611,30 @@ def read_part(self, start, nitems):
 
     def read_full(self):
         return self.chunk_store[self.store_key]
+
+
+def retry_call(callabl: Callable,
+               args=None,
+               kwargs=None,
+               exceptions: Tuple[Any, ...] = (),
+               retries: int = 10,
+               wait: float = 0.1) -> Any:
+    """
+    Make several attempts to invoke the callable. If one of the given exceptions
+    is raised, wait the given period of time and retry up to the given number of
+    retries.
+    """
+
+    if args is None:
+        args = ()
+    if kwargs is None:
+        kwargs = {}
+
+    for attempt in range(1, retries+1):
+        try:
+            return callabl(*args, **kwargs)
+        except exceptions:
+            if attempt < retries:
+                time.sleep(wait)
+            else:
+                raise