[ARVADOS] created: 6bfb4fc09e5b6f1f54c784dd1f7e2c6700020bb3
Git user
git at public.curoverse.com
Fri Sep 22 14:15:36 EDT 2017
at 6bfb4fc09e5b6f1f54c784dd1f7e2c6700020bb3 (commit)
commit 6bfb4fc09e5b6f1f54c784dd1f7e2c6700020bb3
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Fri Sep 22 11:21:07 2017 -0400
12276: Reduce number of spurious invalidations sent to kernel.
* New policy to only send invalidations on objects that have a nonzero
kernel reference count.
* When clearing the contents of a CollectionDirectory, only invalidate the
topmost directory entry, this should take care of invalidating all paths
beneath it as well.
* Don't send invalidate_inode() when deleting an inode (by definition, the
inode is unreferenced by the kernel).
* Remove allow_dirent_cache for now (this only relates to caching negative
lookups, which we don't support).
* Set attribute attr_timeout based on the polling period of the underlying
object.
* FuncToJSONFile sets allow_attr_cache = False instead of allow_dirent_cache =
False (which is useless in this case).
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/fuse/arvados_fuse/__init__.py b/services/fuse/arvados_fuse/__init__.py
index 30770fc..de0338e 100644
--- a/services/fuse/arvados_fuse/__init__.py
+++ b/services/fuse/arvados_fuse/__init__.py
@@ -159,8 +159,8 @@ class InodeCache(object):
if obj.in_use():
_logger.debug("InodeCache cannot clear inode %i, in use", obj.inode)
return
+ obj.kernel_invalidate()
if obj.has_ref(True):
- obj.kernel_invalidate()
_logger.debug("InodeCache sent kernel invalidate inode %i", obj.inode)
return
obj.clear()
@@ -266,17 +266,22 @@ class Inodes(object):
del self._entries[entry.inode]
with llfuse.lock_released:
entry.finalize()
- self.invalidate_inode(entry.inode)
entry.inode = None
else:
entry.dead = True
_logger.debug("del_entry on inode %i with refcount %i", entry.inode, entry.ref_count)
- def invalidate_inode(self, inode):
- llfuse.invalidate_inode(inode)
+ def invalidate_inode(self, entry):
+ if entry.has_ref(False):
+ # Only necessary if the kernel has previously done a lookup on this
+ # inode and hasn't yet forgotten about it.
+ llfuse.invalidate_inode(entry.inode)
- def invalidate_entry(self, inode, name):
- llfuse.invalidate_entry(inode, name.encode(self.encoding))
+ def invalidate_entry(self, entry, name):
+ if entry.has_ref(False):
+ # Only necessary if the kernel has previously done a lookup on this
+ # inode and hasn't yet forgotten about it.
+ llfuse.invalidate_entry(entry.inode, name.encode(self.encoding))
def clear(self):
self.inode_cache.clear()
@@ -432,8 +437,7 @@ class Operations(llfuse.Operations):
entry = llfuse.EntryAttributes()
entry.st_ino = inode
entry.generation = 0
- entry.entry_timeout = 60 if e.allow_dirent_cache else 0
- entry.attr_timeout = 60 if e.allow_attr_cache else 0
+ entry.attr_timeout = e.time_to_next_poll() if e.allow_attr_cache else 0
entry.st_mode = stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH
if isinstance(e, Directory):
diff --git a/services/fuse/arvados_fuse/fresh.py b/services/fuse/arvados_fuse/fresh.py
index a51dd90..8b680f0 100644
--- a/services/fuse/arvados_fuse/fresh.py
+++ b/services/fuse/arvados_fuse/fresh.py
@@ -70,8 +70,9 @@ class FreshBase(object):
self.dead = False
self.cache_size = 0
self.cache_uuid = None
+
+ # Can the kernel cache attributes?
self.allow_attr_cache = True
- self.allow_dirent_cache = True
def invalidate(self):
"""Indicate that object contents should be refreshed from source."""
@@ -142,3 +143,13 @@ class FreshBase(object):
def child_event(self, ev):
pass
+
+ def time_to_next_poll(self):
+ if self._poll:
+ t = (self._last_update + self._poll_time) - self._atime
+ if t < 0:
+ return 0
+ else:
+ return t
+ else:
+ return self._poll_time
diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index 0178fe5..12c02e2 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -150,12 +150,12 @@ class Directory(FreshBase):
# delete any other directory entries that were not in found in 'items'
for i in oldentries:
_logger.debug("Forgetting about entry '%s' on inode %i", i, self.inode)
- self.inodes.invalidate_entry(self.inode, i.encode(self.inodes.encoding))
+ self.inodes.invalidate_entry(self, i)
self.inodes.del_entry(oldentries[i])
changed = True
if changed:
- self.inodes.invalidate_inode(self.inode)
+ self.inodes.invalidate_inode(self)
self._mtime = time.time()
self.fresh()
@@ -182,16 +182,22 @@ class Directory(FreshBase):
self._entries = {}
for n in oldentries:
oldentries[n].clear()
- 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 kernel_invalidate(self):
- for n, e in self._entries.iteritems():
- self.inodes.invalidate_entry(self.inode, n.encode(self.inodes.encoding))
- e.kernel_invalidate()
- self.inodes.invalidate_inode(self.inode)
+ # Invalidating the dentry on the parent implies invalidating all paths
+ # below it as well.
+ parent = self.inodes[self.parent_inode]
+
+ # Find self on the parent in order to invalidate this path.
+ # Calling the public items() method might trigger a refresh,
+ # which we definitely don't want, so read the internal dict directly.
+ for k,v in parent._entries.items():
+ _logger.debug("looking at %s %s, self is %s", k, v, self)
+ if v is self:
+ self.inodes.invalidate_entry(parent, k)
+ break
def mtime(self):
return self._mtime
@@ -266,13 +272,13 @@ class CollectionDirectoryBase(Directory):
elif event == arvados.collection.DEL:
ent = self._entries[name]
del self._entries[name]
- self.inodes.invalidate_entry(self.inode, name.encode(self.inodes.encoding))
+ self.inodes.invalidate_entry(self, name)
self.inodes.del_entry(ent)
elif event == arvados.collection.MOD:
if hasattr(item, "fuse_entry") and item.fuse_entry is not None:
- self.inodes.invalidate_inode(item.fuse_entry.inode)
+ self.inodes.invalidate_inode(item.fuse_entry)
elif name in self._entries:
- self.inodes.invalidate_inode(self._entries[name].inode)
+ self.inodes.invalidate_inode(self._entries[name])
def populate(self, mtime):
self._mtime = mtime
@@ -547,7 +553,7 @@ class TmpCollectionDirectory(CollectionDirectoryBase):
if self.collection_record_file:
with llfuse.lock:
self.collection_record_file.invalidate()
- self.inodes.invalidate_inode(self.collection_record_file.inode)
+ self.inodes.invalidate_inode(self.collection_record_file)
_logger.debug("%s invalidated collection record", self)
def collection_record(self):
@@ -639,6 +645,7 @@ will appear if it exists.
return False
try:
+ e = None
e = self.inodes.add_entry(CollectionDirectory(
self.inode, self.inodes, self.api, self.num_retries, k))
@@ -649,12 +656,13 @@ will appear if it exists.
self.inodes.del_entry(e)
return True
else:
- self.inodes.invalidate_entry(self.inode, k)
+ self.inodes.invalidate_entry(self, k)
self.inodes.del_entry(e)
return False
except Exception as ex:
- _logger.debug('arv-mount exception keep %s', ex)
- self.inodes.del_entry(e)
+ _logger.exception("arv-mount lookup '%s':", k)
+ if e is not None:
+ self.inodes.del_entry(e)
return False
def __getitem__(self, item):
@@ -963,7 +971,7 @@ class ProjectDirectory(Directory):
# Acually move the entry from source directory to this directory.
del src._entries[name_old]
self._entries[name_new] = ent
- self.inodes.invalidate_entry(src.inode, name_old.encode(self.inodes.encoding))
+ self.inodes.invalidate_entry(src, name_old)
@use_counter
def child_event(self, ev):
@@ -1000,7 +1008,7 @@ class ProjectDirectory(Directory):
if old_name in self._entries:
ent = self._entries[old_name]
del self._entries[old_name]
- self.inodes.invalidate_entry(self.inode, old_name.encode(self.inodes.encoding))
+ self.inodes.invalidate_entry(self, old_name)
if new_name:
if ent is not None:
diff --git a/services/fuse/arvados_fuse/fusefile.py b/services/fuse/arvados_fuse/fusefile.py
index 8189a19..5855361 100644
--- a/services/fuse/arvados_fuse/fusefile.py
+++ b/services/fuse/arvados_fuse/fusefile.py
@@ -122,12 +122,11 @@ class FuncToJSONFile(StringFile):
super(FuncToJSONFile, self).__init__(parent_inode, "", 0)
self.func = func
- # invalidate_inode() and invalidate_entry() are asynchronous
- # with no callback to wait for. In order to guarantee
- # userspace programs don't get stale data that was generated
- # before the last invalidate(), we must disallow dirent
+ # invalidate_inode() is asynchronous with no callback to wait for. In
+ # order to guarantee userspace programs don't get stale data that was
+ # generated before the last invalidate(), we must disallow inode
# caching entirely.
- self.allow_dirent_cache = False
+ self.allow_attr_cache = False
def size(self):
self._update()
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list