3198: Inodes() and Operations() take InodeCache() object directly. Minimum

number of entries to retain is configurable.  Added unit test of inode cache.

diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index da6c2e33753854e1d94d1cbded472aae4a3bd09e..2387c699c1e3463bb4846c81471e681664f22728 100644 (file)
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -57,59 +57,61 @@ class DirectoryHandle(object):
 
 
 class InodeCache(object):
-    def __init__(self, cap):
+    def __init__(self, cap, min_entries=4):
         self._entries = collections.OrderedDict()
         self._counter = itertools.count(1)
         self.cap = cap
         self._total = 0
+        self.min_entries = min_entries
+
+    def total(self):
+        return self._total
 
     def _remove(self, obj, clear):
         if clear and not obj.clear():
             _logger.debug("Could not clear %s in_use %s", obj, obj.in_use())
             return False
-        self._total -= obj._cache_size
-        del self._entries[obj._cache_priority]
+        self._total -= obj.cache_size
+        del self._entries[obj.cache_priority]
         _logger.debug("Cleared %s total now %i", obj, self._total)
         return True
 
     def cap_cache(self):
         _logger.debug("total is %i cap is %i", self._total, self.cap)
         if self._total > self.cap:
-            need_gc = False
             for key in list(self._entries.keys()):
-                if self._total < self.cap or len(self._entries) < 4:
+                if self._total < self.cap or len(self._entries) < self.min_entries:
                     break
                 self._remove(self._entries[key], True)
 
-
     def manage(self, obj):
         if obj.persisted():
-            obj._cache_priority = next(self._counter)
-            obj._cache_size = obj.objsize()
-            self._entries[obj._cache_priority] = obj
+            obj.cache_priority = next(self._counter)
+            obj.cache_size = obj.objsize()
+            self._entries[obj.cache_priority] = obj
             self._total += obj.objsize()
             _logger.debug("Managing %s total now %i", obj, self._total)
             self.cap_cache()
 
     def touch(self, obj):
         if obj.persisted():
-            if obj._cache_priority in self._entries:
+            if obj.cache_priority in self._entries:
                 self._remove(obj, False)
             self.manage(obj)
             _logger.debug("Touched %s (%i) total now %i", obj, obj.objsize(), self._total)
 
     def unmanage(self, obj):
-        if obj.persisted() and obj._cache_priority in self._entries:
+        if obj.persisted() and obj.cache_priority in self._entries:
             self._remove(obj, True)
 
 class Inodes(object):
     """Manage the set of inodes.  This is the mapping from a numeric id
     to a concrete File or Directory object"""
 
-    def __init__(self, inode_cache=256*1024*1024):
+    def __init__(self, inode_cache):
         self._entries = {}
         self._counter = itertools.count(llfuse.ROOT_INODE)
-        self._obj_cache = InodeCache(cap=inode_cache)
+        self.inode_cache = inode_cache
 
     def __getitem__(self, item):
         return self._entries[item]
@@ -128,19 +130,16 @@ class Inodes(object):
 
     def touch(self, entry):
         entry._atime = time.time()
-        self._obj_cache.touch(entry)
-
-    def cap_cache(self):
-        self._obj_cache.cap_cache()
+        self.inode_cache.touch(entry)
 
     def add_entry(self, entry):
         entry.inode = next(self._counter)
         self._entries[entry.inode] = entry
-        self._obj_cache.manage(entry)
+        self.inode_cache.manage(entry)
         return entry
 
     def del_entry(self, entry):
-        self._obj_cache.unmanage(entry)
+        self.inode_cache.unmanage(entry)
         llfuse.invalidate_inode(entry.inode)
         del self._entries[entry.inode]
 
@@ -157,7 +156,7 @@ class Operations(llfuse.Operations):
 
     """
 
-    def __init__(self, uid, gid, encoding="utf-8", inode_cache=1000):
+    def __init__(self, uid, gid, encoding="utf-8", inode_cache=InodeCache(cap=256*1024*1024)):
         super(Operations, self).__init__()
 
         self.inodes = Inodes(inode_cache)
@@ -278,7 +277,7 @@ class Operations(llfuse.Operations):
         if fh in self._filehandles:
             self._filehandles[fh].release()
             del self._filehandles[fh]
-        self.inodes.cap_cache()
+        self.inodes.inode_cache.cap_cache()
 
     def releasedir(self, fh):
         self.release(fh)

diff --git a/services/fuse/arvados_fuse/fresh.py b/services/fuse/arvados_fuse/fresh.py
index 9da3a5c912042987cdd6a5f510ff0ebbbd37dabd..5acadfdf7a4fb9b41b6f8c32515a8675cc7e9adf 100644 (file)
--- a/services/fuse/arvados_fuse/fresh.py
+++ b/services/fuse/arvados_fuse/fresh.py
@@ -31,6 +31,8 @@ class FreshBase(object):
         self._atime = time.time()
         self._poll_time = 60
         self.use_count = 0
+        self.cache_priority = 0
+        self.cache_size = 0
 
     # Mark the value as stale
     def invalidate(self):

diff --git a/services/fuse/bin/arv-mount b/services/fuse/bin/arv-mount
index 3c96a5636c8f558f01254ef44a69a7a502ca14fd..a5f97562596eb7f5bd33229c3d2600375160590c 100755 (executable)
--- a/services/fuse/bin/arv-mount
+++ b/services/fuse/bin/arv-mount
@@ -84,7 +84,8 @@ with "--".
 
     try:
         # Create the request handler
-        operations = Operations(os.getuid(), os.getgid(), args.encoding, args.inode_cache)
+        operations = Operations(os.getuid(), os.getgid(), args.encoding,
+                                arvados_fuse.InodeCache(args.inode_cache))
         api = ThreadSafeApiCache(arvados.config.settings())
 
         usr = api.users().current().execute(num_retries=args.retries)

diff --git a/services/fuse/setup.py b/services/fuse/setup.py
index a7ae1c2e0694682857ca60b301e8ab4fb8a8d6fb..0ef3ea67df379806a56820ba273b482d61d0281b 100644 (file)
--- a/services/fuse/setup.py
+++ b/services/fuse/setup.py
@@ -35,7 +35,7 @@ setup(name='arvados_fuse',
         'ciso8601'
         ],
       test_suite='tests',
-      tests_require=['PyYAML'],
+      tests_require=['mock>=1.0', 'PyYAML'],
       zip_safe=False,
       cmdclass={'egg_info': tagger},
       )

diff --git a/services/fuse/tests/test_inodes.py b/services/fuse/tests/test_inodes.py
new file mode 100644 (file)
index 0000000..82bb716
--- /dev/null
+++ b/services/fuse/tests/test_inodes.py
@@ -0,0 +1,87 @@
+import arvados_fuse
+import mock
+import unittest
+
+class InodeTests(unittest.TestCase):
+    def test_inodes(self):
+        cache = arvados_fuse.InodeCache(1000, 4)
+        inodes = arvados_fuse.Inodes(cache)
+
+        # Check that ent1 gets added to inodes
+        ent1 = mock.MagicMock()
+        ent1.return_value.in_use = 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)
+        self.assertIs(inodes[ent1.inode], ent1)
+        self.assertEqual(500, cache.total())
+
+        # ent2 is not persisted, so it doesn't
+        # affect the cache total
+        ent2 = mock.MagicMock()
+        ent2.return_value.in_use = False
+        ent2.persisted.return_value = False
+        ent2.objsize.return_value = 600
+        inodes.add_entry(ent2)
+        self.assertEqual(500, cache.total())
+
+        # ent3 is persisted, adding it should cause ent1 to get cleared
+        ent3 = mock.MagicMock()
+        ent3.return_value.in_use = 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)
+
+        # Won't clear anything because min_entries = 4
+        self.assertEqual(2, len(cache._entries))
+        self.assertFalse(ent1.clear.called)
+        self.assertEqual(1100, cache.total())
+
+        # Change min_entries
+        cache.min_entries = 1
+        cache.cap_cache()
+        self.assertEqual(600, cache.total())
+        self.assertTrue(ent1.clear.called)
+
+        # Touching ent1 should cause ent3 to get cleared
+        self.assertFalse(ent3.clear.called)
+        cache.touch(ent1)
+        self.assertTrue(ent3.clear.called)
+        self.assertEqual(500, cache.total())
+
+        # ent1, ent3 clear return false, can't be cleared
+        ent1.clear.return_value = False
+        ent3.clear.return_value = False
+        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.assertEqual(1100, cache.total())
+
+        # ent1 clear return false, so ent3
+        # gets cleared
+        ent1.clear.return_value = False
+        ent3.clear.return_value = True
+        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.assertEqual(500, cache.total())
+
+        # Delete ent1
+        ent1.clear.return_value = True
+        inodes.del_entry(ent1)
+        self.assertEqual(0, cache.total())
+        cache.touch(ent3)
+        self.assertEqual(600, cache.total())

diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index ecc08888dff6349352ab940908a9d936591d39df..5535494a67b77089d233544e2c2c1e219eedc622 100644 (file)
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -25,7 +25,7 @@ class MountTestBase(unittest.TestCase):
         self.api = arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
 
     def make_mount(self, root_class, **root_kwargs):
-        operations = fuse.Operations(os.getuid(), os.getgid(), inode_cache=2)
+        operations = fuse.Operations(os.getuid(), os.getgid())
         operations.inodes.add_entry(root_class(
             llfuse.ROOT_INODE, operations.inodes, self.api, 0, **root_kwargs))
         llfuse.init(operations, self.mounttmp, [])