[ARVADOS] created: 130cbc5cd46272834c2971b40bdba8c32eeee614
git at public.curoverse.com
git at public.curoverse.com
Mon Jul 27 09:10:15 EDT 2015
at 130cbc5cd46272834c2971b40bdba8c32eeee614 (commit)
commit 130cbc5cd46272834c2971b40bdba8c32eeee614
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Mon Jul 27 09:11:07 2015 -0400
6643: Support associating uuid with multiple inodes to fix double-remove bug.
diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index 59b4a62..775b9be 100644
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -148,7 +148,9 @@ class InodeCache(object):
self._total -= obj.cache_size
del self._entries[obj.cache_priority]
if obj.cache_uuid:
- del self._by_uuid[obj.cache_uuid]
+ self._by_uuid[obj.cache_uuid].remove(obj)
+ if not self._by_uuid[obj.cache_uuid]:
+ del self._by_uuid[obj.cache_uuid]
obj.cache_uuid = None
if clear:
_logger.debug("InodeCache cleared %i total now %i", obj.inode, self._total)
@@ -168,7 +170,11 @@ class InodeCache(object):
self._entries[obj.cache_priority] = obj
obj.cache_uuid = obj.uuid()
if obj.cache_uuid:
- self._by_uuid[obj.cache_uuid] = obj
+ if obj.cache_uuid not in self._by_uuid:
+ self._by_uuid[obj.cache_uuid] = [obj]
+ else:
+ if obj not in self._by_uuid[obj.cache_uuid]:
+ self._by_uuid[obj.cache_uuid].append(obj)
self._total += obj.objsize()
_logger.debug("InodeCache touched %i (size %i) (uuid %s) total now %i", obj.inode, obj.objsize(), obj.cache_uuid, self._total)
self.cap_cache()
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 5ebb6b9..de12fcc 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -524,13 +524,17 @@ will appear if it exists.
self.inode, self.inodes, self.api, self.num_retries, k))
if e.update():
- self._entries[k] = e
+ if k not in self._entries:
+ self._entries[k] = e
+ else:
+ self.inodes.del_entry(e)
return True
else:
- _logger.debug('update failed of %s', k)
+ self.inodes.del_entry(e)
return False
except Exception as e:
_logger.debug('arv-mount exception keep %s', e)
+ self.inodes.del_entry(e)
return False
def __getitem__(self, item):
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 53bde6e..b930974 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -1022,13 +1022,14 @@ class MagicDirApiError(FuseMagicTest):
self.make_mount(fuse.MagicDirectory)
self.operations.inodes.inode_cache.cap = 1
- self.operations.inodes.inode_cache.min_entries = 1
+ self.operations.inodes.inode_cache.min_entries = 2
with self.assertRaises(OSError):
llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
+
class FuseUnitTest(unittest.TestCase):
def test_sanitize_filename(self):
acceptable = [
commit 15ac0086cc6623fa3e4c601c19a14fdcd6c139ca
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Fri Jul 24 17:30:20 2015 -0400
6643: Add test case that reproduces the bug
diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index b24aaa6..59b4a62 100644
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -170,7 +170,7 @@ class InodeCache(object):
if obj.cache_uuid:
self._by_uuid[obj.cache_uuid] = obj
self._total += obj.objsize()
- _logger.debug("InodeCache touched %i (size %i) total now %i", obj.inode, obj.objsize(), self._total)
+ _logger.debug("InodeCache touched %i (size %i) (uuid %s) total now %i", obj.inode, obj.objsize(), obj.cache_uuid, self._total)
self.cap_cache()
else:
obj.cache_priority = None
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 16b3bb2..5ebb6b9 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -423,8 +423,8 @@ class CollectionDirectory(CollectionDirectoryBase):
return True
finally:
self._updating_lock.release()
- except arvados.errors.NotFoundError:
- _logger.exception("arv-mount %s: error", self.collection_locator)
+ except arvados.errors.NotFoundError as e:
+ _logger.error("Error fetching collection '%s': %s", self.collection_locator, e)
except arvados.errors.ArgumentError as detail:
_logger.warning("arv-mount %s: error %s", self.collection_locator, detail)
if self.collection_record is not None and "manifest_text" in self.collection_record:
@@ -527,6 +527,7 @@ will appear if it exists.
self._entries[k] = e
return True
else:
+ _logger.debug('update failed of %s', k)
return False
except Exception as e:
_logger.debug('arv-mount exception keep %s', e)
diff --git a/services/fuse/tests/mount_test_base.py b/services/fuse/tests/mount_test_base.py
index ab623c3..3b7cbaa 100644
--- a/services/fuse/tests/mount_test_base.py
+++ b/services/fuse/tests/mount_test_base.py
@@ -17,7 +17,7 @@ import run_test_server
logger = logging.getLogger('arvados.arv-mount')
class MountTestBase(unittest.TestCase):
- def setUp(self):
+ def setUp(self, api=None):
# The underlying C implementation of open() makes a fstat() syscall
# with the GIL still held. When the GETATTR message comes back to
# llfuse (which in these tests is in the same interpreter process) it
@@ -32,7 +32,7 @@ class MountTestBase(unittest.TestCase):
self.mounttmp = tempfile.mkdtemp()
run_test_server.run()
run_test_server.authorize_with("admin")
- self.api = arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
+ self.api = api if api else arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
def make_mount(self, root_class, **root_kwargs):
self.operations = fuse.Operations(os.getuid(), os.getgid(), enable_write=True)
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index 5e818cc..53bde6e 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -15,6 +15,7 @@ import unittest
import logging
import multiprocessing
import run_test_server
+import mock
from mount_test_base import MountTestBase
@@ -110,8 +111,8 @@ class FuseNoAPITest(MountTestBase):
class FuseMagicTest(MountTestBase):
- def setUp(self):
- super(FuseMagicTest, self).setUp()
+ def setUp(self, api=None):
+ super(FuseMagicTest, self).setUp(api=api)
cw = arvados.CollectionWriter()
@@ -119,7 +120,8 @@ class FuseMagicTest(MountTestBase):
cw.write("data 1")
self.testcollection = cw.finish()
- self.api.collections().create(body={"manifest_text":cw.manifest_text()}).execute()
+ self.test_manifest = cw.manifest_text()
+ self.api.collections().create(body={"manifest_text":self.test_manifest}).execute()
def runTest(self):
self.make_mount(fuse.MagicDirectory)
@@ -1009,6 +1011,24 @@ class FuseFsyncTest(FuseMagicTest):
self.pool.apply(fuseFsyncTestHelper, (self.mounttmp, self.testcollection))
+class MagicDirApiError(FuseMagicTest):
+ def setUp(self):
+ api = mock.MagicMock()
+ super(MagicDirApiError, self).setUp(api=api)
+ api.collections().get().execute.side_effect = iter([Exception('API fail'), {"manifest_text": self.test_manifest}])
+ api.keep.get.side_effect = Exception('Keep fail')
+
+ def runTest(self):
+ self.make_mount(fuse.MagicDirectory)
+
+ self.operations.inodes.inode_cache.cap = 1
+ self.operations.inodes.inode_cache.min_entries = 1
+
+ with self.assertRaises(OSError):
+ llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
+
+ llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
+
class FuseUnitTest(unittest.TestCase):
def test_sanitize_filename(self):
acceptable = [
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list