named caches: move instead of symlinking

client/named_cache.py

 # Copyright 2016 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """This file implements Named Caches."""
 
 import contextlib
 import logging
 import optparse
 import os
@@ skipping 35 matching lines @@
     """
     assert file_path.isabs(root_dir), root_dir
     self.root_dir = unicode(root_dir)
     self._lock = threading_utils.LockWithAssert()
     # LRU {cache_name -> cache_location}
     # It is saved to |root_dir|/state.json.
     self._lru = None
 
   @contextlib.contextmanager
   def open(self, time_fn=None):
-    """Opens NamedCaches for mutation operations, such as request or trim.
+    """Opens NamedCaches for mutation operations, such as install.
 
-    Only on caller can open the cache manager at a time. If the same thread
+    Only one caller can open the cache manager at a time. If the same thread
     calls this function after opening it earlier, the call will deadlock.
 
     time_fn is a function that returns timestamp (float) and used to take
     timestamps when new caches are requested.
 
     Returns a context manager that must be closed as soon as possible.
     """
     with self._lock:
       state_path = os.path.join(self.root_dir, u'state.json')
       assert self._lru is None, 'acquired lock, but self._lru is not None'
@@ skipping 10 matching lines @@
       try:
         yield
       finally:
         file_path.ensure_tree(self.root_dir)
         self._lru.save(state_path)
         self._lru = None
 
   def __len__(self):
     """Returns number of items in the cache.
 
-    Requires NamedCache to be open.
+    NamedCache must be open.
     """
     return len(self._lru)
 
-  def request(self, name):
-    """Returns an absolute path to the directory of the named cache.
-
-    Creates a cache directory if it does not exist yet.
-
-    Requires NamedCache to be open.
-    """
-    self._lock.assert_locked()
-    assert isinstance(name, basestring), name
-    path = self._lru.get(name)
-    create_named_link = False
-    if path is None:
-      path = self._allocate_dir()
-      create_named_link = True
-      logging.info('Created %r for %r', path, name)
-    abs_path = os.path.join(self.root_dir, path)
-
-    # TODO(maruel): That's weird, it should exist already.
-    file_path.ensure_tree(abs_path)
-    self._lru.add(name, path)
-
-    if create_named_link:
-      # Create symlink <root_dir>/<named>/<name> -> <root_dir>/<short name>
-      # for user convenience.
-      named_path = self._get_named_path(name)
-      if os.path.exists(named_path):
-        file_path.remove(named_path)
-      else:
-        file_path.ensure_tree(os.path.dirname(named_path))
-      logging.info('Symlink %r to %r', named_path, abs_path)
-      fs.symlink(abs_path, named_path)
-
-    return abs_path
-
   def get_oldest(self):
     """Returns name of the LRU cache or None.
 
-    Requires NamedCache to be open.
+    NamedCache must be open..

[M-A Ruel, 2017/05/09 18:35:14]

one dot

[nodir, 2017/05/09 23:20:09]

Done.

     """
     self._lock.assert_locked()
     try:
       return self._lru.get_oldest()[0]
     except KeyError:
       return None
 
   def get_timestamp(self, name):
     """Returns timestamp of last use of an item.
 
-    Requires NamedCache to be open.
+    NamedCache must be open.
 
     Raises KeyError if cache is not found.
     """
     self._lock.assert_locked()
     assert isinstance(name, basestring), name
     return self._lru.get_timestamp(name)
 
-  @contextlib.contextmanager
-  def create_symlinks(self, root, named_caches):
-    """Creates symlinks in |root| for the specified named_caches.
+  @property
+  def available(self):
+    """Returns a set of names of available caches.
+
+    NamedCache must be open.
+    """
+    self._lock.assert_locked()
+    return self._lru.keys_set()
+
+  def install(self, root, named_caches):

[M-A Ruel, 2017/05/09 18:35:14]

I'd prefer to have this function accept one named_cache, the loop should be
outside.

[nodir, 2017/05/09 23:20:09]

do'h! Yeah, this is much simpler

+    """Moves directories of the specified named caches to |root|.
 
     named_caches must be a list of (name, path) tuples.
 
-    Requires NamedCache to be open.
+    NamedCache must be open..

[M-A Ruel, 2017/05/09 18:35:14]

same

[nodir, 2017/05/09 23:20:09]

Done.

 
     Raises Error if cannot create a symlink.
     """
     self._lock.assert_locked()
-    for name, path in named_caches:
-      logging.info('Named cache %r -> %r', name, path)
+    for name, rel_install in named_caches:
+      logging.info('Installing named cache %r to %r', name, rel_install)
       try:
-        _validate_named_cache_path(path)
-        symlink_path = os.path.abspath(os.path.join(root, path))
-        file_path.ensure_tree(os.path.dirname(symlink_path))
-        requested = self.request(name)
-        logging.info('Symlink %r to %r', symlink_path, requested)
-        fs.symlink(requested, symlink_path)
+        _validate_named_cache_path(rel_install)
+        rel_install = unicode(rel_install)
+        abs_install = os.path.abspath(os.path.join(root, rel_install))
+
+        rel_cache = self._lru.get(name)
+        if rel_cache:
+          abs_cache = os.path.join(self.root_dir, rel_cache)
+          if os.path.isdir(abs_cache):
+            logging.info('Moving %r to %r', abs_cache, abs_install)
+            file_path.ensure_tree(os.path.dirname(abs_install))
+            fs.rename(abs_cache, abs_install)
+            self._remove(name)
+            continue
+
+          logging.warning('directory for named cache %r does not exist', name)
+          self._remove(name)
+
+        # The named cache does not exist.
+        # Just create an empty directory in the root.
+        # When uninstalling, we will move it back to the cache and create an
+        # an entry.
+        file_path.ensure_tree(abs_install)
       except (OSError, Error) as ex:
         raise Error(
-            'cannot create a symlink for cache named "%s" at "%s": %s' % (
-              name, symlink_path, ex))
+            'cannot install cache named %r at %r: %s' % (
+              name, abs_install, ex))
 
-  def delete_symlinks(self, root, named_caches):
-    """Deletes symlinks from |root| for the specified named_caches.
+  def uninstall(self, root, named_caches):

[M-A Ruel, 2017/05/09 18:35:14]

same about one call per named cache. Makes the function more readable.

[nodir, 2017/05/09 23:20:09]

Done.

+    """Moves directories of the specified named caches back from |root|.
 
     named_caches must be a list of (name, path) tuples.
     """
-    for name, path in named_caches:
-      logging.info('Unlinking named cache "%s"', name)
+    for name, rel_install in named_caches:
+      logging.info('Uninstalling named cache %r from %r', name, rel_install)
       try:
-        _validate_named_cache_path(path)
-        symlink_path = os.path.abspath(os.path.join(root, path))
-        fs.unlink(symlink_path)
+        _validate_named_cache_path(rel_install)
+        rel_install = unicode(rel_install)

[M-A Ruel, 2017/05/09 18:35:14]

don't, make sure caller is fixed.

[nodir, 2017/05/09 23:20:09]

Done.

+        abs_install = os.path.abspath(os.path.join(root, rel_install))
+        if not os.path.isdir(abs_install):
+          logging.warning(
+              'Directory %r does not exist anymore. Cache lost.', abs_install)
+          continue
+
+        rel_cache = self._lru.get(name)
+        if rel_cache:
+          # Do not crash because cache already exists.
+          logging.warning('overwriting an existing named cache %r', name)
+          create_named_link = False
+        else:
+          rel_cache = self._allocate_dir()
+          create_named_link = True
+
+        # Move the dir and create an entry for the named cache.
+        abs_cache = os.path.join(self.root_dir, rel_cache)
+        logging.info('Moving %r to %r', abs_install, abs_cache)
+        file_path.ensure_tree(os.path.dirname(abs_cache))
+        fs.rename(abs_install, abs_cache)
+        self._lru.add(name, rel_cache)
+
+        if create_named_link:
+          # Create symlink <root_dir>/<named>/<name> -> <root_dir>/<short name>
+          # for user convenience.
+          named_path = self._get_named_path(name)
+          if os.path.exists(named_path):
+            file_path.remove(named_path)
+          else:
+            file_path.ensure_tree(os.path.dirname(named_path))
+          fs.symlink(abs_cache, named_path)
+          logging.info('Created symlink %r for %r', named_path, name)
       except (OSError, Error) as ex:
         raise Error(
-            'cannot unlink cache named "%s" at "%s": %s' % (
-              name, symlink_path, ex))
+            'cannot uninstall cache named %r at %r: %s' % (
+              name, abs_install, ex))
 
   def trim(self, min_free_space):
     """Purges cache.
 
     Removes cache directories that were not accessed for a long time
     until there is enough free space and the number of caches is sane.
 
     If min_free_space is None, disk free space is not checked.
 
-    Requires NamedCache to be open.
+    NamedCache must be open..

[M-A Ruel, 2017/05/09 18:35:14]

same

 
     Returns:
       Number of caches deleted.
     """
     self._lock.assert_locked()
     if not os.path.isdir(self.root_dir):
       return 0
 
     total = 0
     free_space = 0
     if min_free_space:
       free_space = file_path.get_free_space(self.root_dir)
     while ((min_free_space and free_space < min_free_space)
            or len(self._lru) > MAX_CACHE_SIZE):
       logging.info(
-          'Making space for named cache %s > %s or %s > %s',
+          'Making space for named cache %d > %d or %d > %d',
           free_space, min_free_space, len(self._lru), MAX_CACHE_SIZE)
       try:
         name, (path, _) = self._lru.get_oldest()
       except KeyError:
         return total
-      named_dir = self._get_named_path(name)
-      if fs.islink(named_dir):
-        fs.unlink(named_dir)
-      path_abs = os.path.join(self.root_dir, path)
-      if os.path.isdir(path_abs):
-        logging.info('Removing named cache %s', path_abs)
-        file_path.rmtree(path_abs)
+      logging.info('Removing named cache %r', name)
+      self._remove(name)
       if min_free_space:
         free_space = file_path.get_free_space(self.root_dir)
-      self._lru.pop(name)
       total += 1
     return total
 
   _DIR_ALPHABET = string.ascii_letters + string.digits
 
   def _allocate_dir(self):
     """Creates and returns relative path of a new cache directory."""
     # We randomly generate directory names that have two lower/upper case
     # letters or digits. Total number of possibilities is (26*2 + 10)^2 = 3844.
     abc_len = len(self._DIR_ALPHABET)
     tried = set()
     while len(tried) < 1000:
       i = random.randint(0, abc_len * abc_len - 1)
       rel_path = (
         self._DIR_ALPHABET[i / abc_len] +
         self._DIR_ALPHABET[i % abc_len])
       if rel_path in tried:
         continue
       abs_path = os.path.join(self.root_dir, rel_path)
       if not fs.exists(abs_path):
         return rel_path
       tried.add(rel_path)
     raise Error('could not allocate a new cache dir, too many cache dirs')
 
+  def _remove(self, name):
+    """Removes a cache directory and entry.
+
+    NamedCache must be open.
+
+    Returns:
+      Number of caches deleted.
+    """
+    self._lock.assert_locked()
+    rel_path = self._lru.get(name)
+    if not rel_path:
+      return
+
+    named_dir = self._get_named_path(name)
+    if fs.islink(named_dir):
+      fs.unlink(named_dir)
+
+    abs_path = os.path.join(self.root_dir, rel_path)
+    if os.path.isdir(abs_path):
+      file_path.rmtree(abs_path)
+    self._lru.pop(name)
+
   def _get_named_path(self, name):
     return os.path.join(self.root_dir, 'named', name)
 
 
 def add_named_cache_options(parser):
   group = optparse.OptionGroup(parser, 'Named caches')
   group.add_option(
       '--named-cache',
       dest='named_caches',
       action='append',
@@ skipping 10 matching lines @@
   parser.add_option_group(group)
 
 
 def process_named_cache_options(parser, options):
   """Validates named cache options and returns a CacheManager."""
   if options.named_caches and not options.named_cache_root:
     parser.error('--named-cache is specified, but --named-cache-root is empty')
   for name, path in options.named_caches:
     if not CACHE_NAME_RE.match(name):
       parser.error(
-          'cache name "%s" does not match %s' % (name, CACHE_NAME_RE.pattern))
+          'cache name %r does not match %r' % (name, CACHE_NAME_RE.pattern))
     if not path:
       parser.error('cache path cannot be empty')
   if options.named_cache_root:
     return CacheManager(os.path.abspath(options.named_cache_root))
   return None
 
 
 def _validate_named_cache_path(path):
+  if not isinstance(path, basestring):
+    raise Error('named cache path must be a string')
   if os.path.isabs(path):
     raise Error('named cache path must not be absolute')
   if '..' in path.split(os.path.sep):
     raise Error('named cache path must not contain ".."')