launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/trivial into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #951401 in Launchpad itself: "parse-ppa-apache-logs failing (missing files)"
  https://bugs.launchpad.net/launchpad/+bug/951401
  Bug #1263002 in Launchpad itself: "Twisted feature flag support fails typecasting when updating"
  https://bugs.launchpad.net/launchpad/+bug/1263002

For more details, see:
https://code.launchpad.net/~stub/launchpad/trivial/+merge/244833

Add an option to rename Librarian disk files when copied into Swift.

We already have options to keep the files (what we are running now), and remove them after copying into Swift (where we would like to be). Turning on the 'remove' option is scary though, so instead lets just rename them. Once renamed, files are effectively removed as none of the the systems will find them. However, we can easily revert if there are problems with a few minutes at the shell prompt.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/trivial into lp:launchpad.

=== modified file 'cronscripts/librarian-feed-swift.py'
--- cronscripts/librarian-feed-swift.py	2014-10-22 10:00:03 +0000
+++ cronscripts/librarian-feed-swift.py	2014-12-16 09:25:02 +0000
@@ -23,9 +23,12 @@
             "-i", "--id", action="append", dest="ids", default=[],
             metavar="CONTENT_ID", help="Migrate a single file")
         self.parser.add_option(
-            "-r", "--remove", action="store_true", default=False,
+            "--remove", action="store_true", default=False,
             help="Remove files from disk after migration (default: False)")
         self.parser.add_option(
+            "--rename", action="store_true", default=False,
+            help="Rename files on disk after migration (default: False)")
+        self.parser.add_option(
             "-s", "--start", action="store", type=int, default=None,
             dest="start", metavar="CONTENT_ID",
             help="Migrate files starting from CONTENT_ID")
@@ -39,22 +42,33 @@
             help="Migrate files up to and including CONTENT_ID")
 
     def main(self):
+        if self.options.rename and self.options.remove:
+            self.parser.error("Cannot both remove and rename")
+        elif self.options.rename:
+            remove = swift.rename
+        elif self.options.remove:
+            remove = os.unlink
+        else:
+            remove = None
+
         if self.options.start_since:
             self.options.start = ISlaveStore(LibraryFileContent).execute("""
                 SELECT MAX(id) FROM LibraryFileContent
                 WHERE datecreated < current_timestamp at time zone 'UTC'
                     - CAST(%s AS INTERVAL)
                 """, (unicode(self.options.start_since),)).get_one()[0]
+
         if self.options.ids and (self.options.start or self.options.end):
             self.parser.error(
                 "Cannot specify both individual file(s) and range")
+
         elif self.options.ids:
             for lfc in self.options.ids:
-                swift.to_swift(self.logger, lfc, lfc, self.options.remove)
+                swift.to_swift(self.logger, lfc, lfc, remove)
+
         else:
-            swift.to_swift(
-                self.logger, self.options.start, self.options.end,
-                self.options.remove)
+            swift.to_swift(self.logger, self.options.start,
+                           self.options.end, remove)
         self.logger.info('Done')
 
 

=== modified file 'lib/lp/services/librarianserver/librariangc.py'
--- lib/lp/services/librarianserver/librariangc.py	2014-10-22 19:52:40 +0000
+++ lib/lp/services/librarianserver/librariangc.py	2014-12-16 09:25:02 +0000
@@ -433,7 +433,7 @@
             "SELECT COALESCE(max(id),0) FROM UnreferencedLibraryFileAlias")
         self.max_id = cur.fetchone()[0]
         log.debug(
-            "%d unreferenced LibraryFileContent to remove." % self.max_id)
+            "%d unreferenced LibraryFileAlias to remove." % self.max_id)
         con.commit()
 
     def isDone(self):
@@ -627,7 +627,8 @@
             dirnames.remove('incoming')
         if 'lost+found' in dirnames:
             dirnames.remove('lost+found')
-        filenames = set(filenames)
+        filenames = set([fn for fn in filenames
+                         if not fn.endswith('.migrated'])
         filenames.discard('librarian.pid')
         filenames.discard('librarian.log')
 

=== modified file 'lib/lp/services/librarianserver/swift.py'
--- lib/lp/services/librarianserver/swift.py	2014-10-15 07:52:40 +0000
+++ lib/lp/services/librarianserver/swift.py	2014-12-16 09:25:02 +0000
@@ -29,13 +29,13 @@
 ONE_DAY = 24 * 60 * 60
 
 
-def to_swift(log, start_lfc_id=None, end_lfc_id=None, remove=False):
+def to_swift(log, start_lfc_id=None, end_lfc_id=None, remove_func=False):
     '''Copy a range of Librarian files from disk into Swift.
 
     start and end identify the range of LibraryFileContent.id to
     migrate (inclusive).
 
-    If remove is True, files are removed from disk after being copied into
+    If remove_func is set, it is called for every file after being copied into
     Swift.
     '''
     swift_connection = connection_pool.get()
@@ -45,7 +45,7 @@
         start_lfc_id = 1
     if end_lfc_id is None:
         # Maximum id capable of being stored on the filesystem - ffffffff
-        end_lfc_id = 4294967295
+        end_lfc_id = 0xffffffff
 
     log.info("Walking disk store {0} from {1} to {2}, inclusive".format(
         fs_root, start_lfc_id, end_lfc_id))
@@ -143,8 +143,15 @@
                     lfc, container, obj_name))
                 _put(log, swift_connection, lfc, container, obj_name, fs_path)
 
-            if remove:
-                os.unlink(fs_path)
+            if remove_func:
+                remove_func(fs_path)
+
+
+def rename(path):
+    # It would be nice to move the file out of the tree entirely, but we
+    # need to keep the backup on the same filesystem as the original
+    # file.
+    os.rename(path, path + '.migrated')
 
 
 def _put(log, swift_connection, lfc_id, container, obj_name, fs_path):