[ARVADOS] updated: bb7f064243fcd052e17815129fb1dba2f86d493d

Git user git at public.curoverse.com
Wed Sep 21 14:17:57 EDT 2016


Summary of changes:
 services/fuse/arvados_fuse/__init__.py | 26 ++++++++++++--------
 services/fuse/arvados_fuse/fresh.py    |  4 ++--
 services/fuse/arvados_fuse/fusedir.py  | 44 ++++++++++++++++++++--------------
 services/fuse/tests/test_cache.py      |  5 +++-
 services/fuse/tests/test_inodes.py     | 40 +++++++++++++++----------------
 5 files changed, 67 insertions(+), 52 deletions(-)

       via  bb7f064243fcd052e17815129fb1dba2f86d493d (commit)
      from  3a391d6693d89c0cfe5862227fcc9a0568bc775d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit bb7f064243fcd052e17815129fb1dba2f86d493d
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Wed Sep 21 14:17:51 2016 -0400

    10117: Additional refinement: a directory not in use but still referenced needs
    to be invalidated before it can be cleared so that the kernel releases its
    references.

diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index ac6e236..e679a23 100644
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -143,9 +143,15 @@ class InodeCache(object):
         return self._total
 
     def _remove(self, obj, clear):
-        if clear and not obj.can_clear():
-            _logger.debug("InodeCache cannot clear %i", obj.inode)
-            return False
+        if clear:
+            if obj.in_use():
+                _logger.debug("InodeCache cannot clear inode %i, in use", obj.inode)
+                return False
+            if obj.has_ref(only_children=True):
+                obj.invalidate()
+                _logger.debug("InodeCache invalidate inode %i", obj.inode)
+                return False
+            obj.clear()
         self._total -= obj.cache_size
         del self._entries[obj.cache_priority]
         if obj.cache_uuid:
@@ -154,7 +160,7 @@ class InodeCache(object):
                 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)
+            _logger.debug("InodeCache cleared inode %i total now %i", obj.inode, self._total)
         return True
 
     def cap_cache(self):
@@ -177,7 +183,7 @@ class InodeCache(object):
                     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)
+            _logger.debug("InodeCache touched inode %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
@@ -632,7 +638,7 @@ class Operations(llfuse.Operations):
 
     @catch_exceptions
     def create(self, inode_parent, name, mode, flags, ctx):
-        _logger.debug("arv-mount create: %i '%s' %o", inode_parent, name, mode)
+        _logger.debug("arv-mount create: parent_inode %i '%s' %o", inode_parent, name, mode)
 
         p = self._check_writable(inode_parent)
         p.create(name)
@@ -648,7 +654,7 @@ class Operations(llfuse.Operations):
 
     @catch_exceptions
     def mkdir(self, inode_parent, name, mode, ctx):
-        _logger.debug("arv-mount mkdir: %i '%s' %o", inode_parent, name, mode)
+        _logger.debug("arv-mount mkdir: parent_inode %i '%s' %o", inode_parent, name, mode)
 
         p = self._check_writable(inode_parent)
         p.mkdir(name)
@@ -661,19 +667,19 @@ class Operations(llfuse.Operations):
 
     @catch_exceptions
     def unlink(self, inode_parent, name):
-        _logger.debug("arv-mount unlink: %i '%s'", inode_parent, name)
+        _logger.debug("arv-mount unlink: parent_inode %i '%s'", inode_parent, name)
         p = self._check_writable(inode_parent)
         p.unlink(name)
 
     @catch_exceptions
     def rmdir(self, inode_parent, name):
-        _logger.debug("arv-mount rmdir: %i '%s'", inode_parent, name)
+        _logger.debug("arv-mount rmdir: parent_inode %i '%s'", inode_parent, name)
         p = self._check_writable(inode_parent)
         p.rmdir(name)
 
     @catch_exceptions
     def rename(self, inode_parent_old, name_old, inode_parent_new, name_new):
-        _logger.debug("arv-mount rename: %i '%s' %i '%s'", inode_parent_old, name_old, inode_parent_new, name_new)
+        _logger.debug("arv-mount rename: old_parent_inode %i '%s' new_parent_inode %i '%s'", inode_parent_old, name_old, inode_parent_new, name_new)
         src = self._check_writable(inode_parent_old)
         dest = self._check_writable(inode_parent_new)
         dest.rename(name_old, name_new, src)
diff --git a/services/fuse/arvados_fuse/fresh.py b/services/fuse/arvados_fuse/fresh.py
index 635acea..95dde2b 100644
--- a/services/fuse/arvados_fuse/fresh.py
+++ b/services/fuse/arvados_fuse/fresh.py
@@ -112,8 +112,8 @@ class FreshBase(object):
         self.ref_count -= n
         return self.ref_count
 
-    def can_clear(self):
-        return not (self.use_count > 0 or self.ref_count > 0)
+    def has_ref(self, only_children=False):
+        return self.ref_count > 0
 
     def objsize(self):
         return 0
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index aa9c4fd..38fe32c 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -156,13 +156,21 @@ class Directory(FreshBase):
 
         self.fresh()
 
-    def can_clear(self):
-        if not super(Directory, self).can_clear():
-            return False
+    def in_use(self):
+        if super(Directory, self).in_use():
+            return True
         for v in self._entries.itervalues():
-            if not v.can_clear():
-                return False
-        return True
+            if v.in_use():
+                return True
+        return False
+
+    def has_ref(self, only_children=False):
+        if not only_children and super(Directory, self).has_ref():
+            return True
+        for v in self._entries.itervalues():
+            if v.has_ref():
+                return True
+        return False
 
     def clear(self):
         """Delete all entries"""
@@ -170,12 +178,21 @@ class Directory(FreshBase):
         self._entries = {}
         for n in oldentries:
             oldentries[n].clear()
-        for n in oldentries:
             self.inodes.invalidate_entry(self.inode, n.encode(self.inodes.encoding))
             self.inodes.del_entry(oldentries[n])
         self.inodes.invalidate_inode(self.inode)
         self.invalidate()
 
+    def invalidate(self):
+        try:
+            super(Directory, self).invalidate()
+            for n, e in self._entries.iteritems():
+                self.inodes.invalidate_entry(self.inode, n.encode(self.inodes.encoding))
+                e.invalidate()
+            self.inodes.invalidate_inode(self.inode)
+        except Exception:
+            _logger.exception()
+
     def mtime(self):
         return self._mtime
 
@@ -324,6 +341,7 @@ class CollectionDirectoryBase(Directory):
     def clear(self):
         r = super(CollectionDirectoryBase, self).clear()
         self.collection = None
+        self._manifest_size = 0
         return r
 
 
@@ -648,17 +666,7 @@ will appear if it exists.
         return not self.pdh_only
 
 
-class RecursiveInvalidateDirectory(Directory):
-    def invalidate(self):
-        try:
-            super(RecursiveInvalidateDirectory, self).invalidate()
-            for a in self._entries:
-                self._entries[a].invalidate()
-        except Exception:
-            _logger.exception()
-
-
-class TagsDirectory(RecursiveInvalidateDirectory):
+class TagsDirectory(Directory):
     """A special directory that contains as subdirectories all tags visible to the user."""
 
     def __init__(self, parent_inode, inodes, api, num_retries, poll_time=60):
diff --git a/services/fuse/tests/test_cache.py b/services/fuse/tests/test_cache.py
index 7aa0009..66b70c9 100644
--- a/services/fuse/tests/test_cache.py
+++ b/services/fuse/tests/test_cache.py
@@ -11,8 +11,11 @@ import unittest
 from .integration_test import IntegrationTest
 from .mount_test_base import MountTestBase
 
+_logger = logging.getLogger('arvados.arvados_fuse')
+_logger.setLevel(logging.DEBUG)
+
 class TmpCollectionTest(IntegrationTest):
-    mnt_args = ["--directory-cache=0"]
+    mnt_args = ["--by-id", "--directory-cache=0"]
 
     @IntegrationTest.mount(argv=mnt_args)
     def test_cache_spill(self):
diff --git a/services/fuse/tests/test_inodes.py b/services/fuse/tests/test_inodes.py
index 61170d5..15bd8a9 100644
--- a/services/fuse/tests/test_inodes.py
+++ b/services/fuse/tests/test_inodes.py
@@ -2,6 +2,7 @@ import arvados_fuse
 import mock
 import unittest
 import llfuse
+import logging
 
 class InodeTests(unittest.TestCase):
     def test_inodes_basic(self):
@@ -11,8 +12,8 @@ class InodeTests(unittest.TestCase):
         # Check that ent1 gets added to inodes
         ent1 = mock.MagicMock()
         ent1.in_use.return_value = False
+        ent1.has_ref.return_value = False
         ent1.persisted.return_value = True
-        ent1.clear.return_value = True
         ent1.objsize.return_value = 500
         inodes.add_entry(ent1)
         self.assertIn(ent1.inode, inodes)
@@ -25,8 +26,8 @@ class InodeTests(unittest.TestCase):
 
         ent1 = mock.MagicMock()
         ent1.in_use.return_value = False
+        ent1.has_ref.return_value = False
         ent1.persisted.return_value = True
-        ent1.clear.return_value = True
         ent1.objsize.return_value = 500
         inodes.add_entry(ent1)
 
@@ -34,6 +35,7 @@ class InodeTests(unittest.TestCase):
         # affect the cache total
         ent2 = mock.MagicMock()
         ent2.in_use.return_value = False
+        ent2.has_ref.return_value = False
         ent2.persisted.return_value = False
         ent2.objsize.return_value = 600
         inodes.add_entry(ent2)
@@ -46,17 +48,17 @@ class InodeTests(unittest.TestCase):
         # Check that ent1 gets added to inodes
         ent1 = mock.MagicMock()
         ent1.in_use.return_value = False
+        ent1.has_ref.return_value = False
         ent1.persisted.return_value = True
-        ent1.clear.return_value = True
         ent1.objsize.return_value = 500
         inodes.add_entry(ent1)
 
         # ent3 is persisted, adding it should cause ent1 to get cleared
         ent3 = mock.MagicMock()
         ent3.in_use.return_value = False
+        ent3.has_ref.return_value = False
         ent3.persisted.return_value = True
         ent3.objsize.return_value = 600
-        ent3.clear.return_value = True
 
         self.assertFalse(ent1.clear.called)
         inodes.add_entry(ent3)
@@ -78,46 +80,43 @@ class InodeTests(unittest.TestCase):
         self.assertTrue(ent3.clear.called)
         self.assertEqual(500, cache.total())
 
-    def test_clear_false(self):
+    def test_clear_in_use(self):
         cache = arvados_fuse.InodeCache(1000, 4)
         inodes = arvados_fuse.Inodes(cache)
 
         ent1 = mock.MagicMock()
-        ent1.in_use.return_value = False
+        ent1.in_use.return_value = True
+        ent1.has_ref.return_value = False
         ent1.persisted.return_value = True
-        ent1.clear.return_value = True
         ent1.objsize.return_value = 500
         inodes.add_entry(ent1)
 
         ent3 = mock.MagicMock()
         ent3.in_use.return_value = False
+        ent3.has_ref.return_value = True
         ent3.persisted.return_value = True
         ent3.objsize.return_value = 600
-        ent3.clear.return_value = True
         inodes.add_entry(ent3)
 
         cache.min_entries = 1
 
-        # ent1, ent3 clear return false, can't be cleared
-        ent1.clear.return_value = False
-        ent3.clear.return_value = False
+        # ent1, ent3 in use, has ref, can't be cleared
         ent1.clear.called = False
         ent3.clear.called = False
         self.assertFalse(ent1.clear.called)
         self.assertFalse(ent3.clear.called)
         cache.touch(ent3)
-        self.assertTrue(ent1.clear.called)
-        self.assertTrue(ent3.clear.called)
+        self.assertFalse(ent1.clear.called)
+        self.assertFalse(ent3.clear.called)
         self.assertEqual(1100, cache.total())
 
-        # ent1 clear return false, so ent3
-        # gets cleared
-        ent1.clear.return_value = False
-        ent3.clear.return_value = True
+        # ent1 still in use, ent3 doesn't have ref,
+        # so ent3 gets cleared
+        ent3.has_ref.return_value = False
         ent1.clear.called = False
         ent3.clear.called = False
         cache.touch(ent3)
-        self.assertTrue(ent1.clear.called)
+        self.assertFalse(ent1.clear.called)
         self.assertTrue(ent3.clear.called)
         self.assertEqual(500, cache.total())
 
@@ -127,20 +126,19 @@ class InodeTests(unittest.TestCase):
 
         ent1 = mock.MagicMock()
         ent1.in_use.return_value = False
+        ent1.has_ref.return_value = False
         ent1.persisted.return_value = True
-        ent1.clear.return_value = True
         ent1.objsize.return_value = 500
         inodes.add_entry(ent1)
 
         ent3 = mock.MagicMock()
         ent3.in_use.return_value = False
+        ent3.has_ref.return_value = False
         ent3.persisted.return_value = True
         ent3.objsize.return_value = 600
-        ent3.clear.return_value = True
 
         # Delete ent1
         self.assertEqual(500, cache.total())
-        ent1.clear.return_value = True
         ent1.ref_count = 0
         with llfuse.lock:
             inodes.del_entry(ent1)

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list