storage.py: more mutable-slot coverage, renewing/cancelling leases

src/allmydata/storage.py

@@ -520,7 +520,7 @@ class MutableShareFile(Referenceable):
         if modified:
             freed_space = self._pack_leases(f)
             f.close()
-            return (freed_space, remaining)
+            return (remaining, freed_space)
         msg = ("Unable to cancel non-existent lease. I have leases "
                "accepted by nodeids: ")
         msg += ",".join([("'%s'" % idlib.b2a(anid))
@@ -720,7 +720,18 @@ class StorageServer(service.MultiService, Referenceable):
         found_buckets = False
         for shnum, filename in self._get_bucket_shares(storage_index):
             found_buckets = True
-            sf = ShareFile(filename)
+            f = open(filename, 'rb')
+            header = f.read(32)
+            f.close()
+            if header[:32] == MutableShareFile.MAGIC:
+                sf = MutableShareFile(filename)
+                # note: if the share has been migrated, the renew_lease()
+                # call will throw an exception, with information to help the
+                # client update the lease.
+            elif header[:4] == struct.pack(">L", 1):
+                sf = ShareFile(filename)
+            else:
+                pass # non-sharefile
             sf.renew_lease(renew_secret, new_expire_time)
         if not found_buckets:
             raise IndexError("no such lease to renew")
@@ -737,7 +748,18 @@ class StorageServer(service.MultiService, Referenceable):
             # (perhaps we ran out of disk space while adding a lease), the
             # leases on all shares will be identical.
             found_buckets = True
-            sf = ShareFile(filename)
+            f = open(filename, 'rb')
+            header = f.read(32)
+            f.close()
+            if header[:32] == MutableShareFile.MAGIC:
+                sf = MutableShareFile(filename)
+                # note: if the share has been migrated, the renew_lease()
+                # call will throw an exception, with information to help the
+                # client update the lease.
+            elif header[:4] == struct.pack(">L", 1):
+                sf = ShareFile(filename)
+            else:
+                pass # non-sharefile
             # this raises IndexError if the lease wasn't present
             remaining_leases, space_freed = sf.cancel_lease(cancel_secret)
             total_space_freed += space_freed

src/allmydata/test/test_storage.py

@@ -466,10 +466,16 @@ class MutableServer(unittest.TestCase):
     def write_enabler(self, we_tag):
         return hashutil.tagged_hash("we_blah", we_tag)
 
+    def renew_secret(self, tag):
+        return hashutil.tagged_hash("renew_blah", str(tag))
+
+    def cancel_secret(self, tag):
+        return hashutil.tagged_hash("cancel_blah", str(tag))
+
     def allocate(self, ss, storage_index, we_tag, lease_tag, sharenums, size):
         write_enabler = self.write_enabler(we_tag)
-        renew_secret = hashutil.tagged_hash("blah", str(lease_tag))
-        cancel_secret = hashutil.tagged_hash("blah", str(lease_tag))
+        renew_secret = self.renew_secret(lease_tag)
+        cancel_secret = self.cancel_secret(lease_tag)
         return ss.remote_allocate_mutable_slot(storage_index,
                                                write_enabler,
                                                renew_secret, cancel_secret,
@@ -697,6 +703,9 @@ class MutableServer(unittest.TestCase):
         # the lease
         shares2 = self.allocate(ss, "si1", "we1", secret, set([0,1,2]), 100)
 
+        # renew it directly
+        ss.remote_renew_lease("si1", self.renew_secret(secret))
+
         # now allocate them with a bunch of different secrets, to trigger the
         # extended lease code
         shares2 = self.allocate(ss, "si1", "we1", secret+1, set([0,1,2]), 100)
@@ -704,6 +713,8 @@ class MutableServer(unittest.TestCase):
         shares2 = self.allocate(ss, "si1", "we1", secret+3, set([0,1,2]), 100)
         shares2 = self.allocate(ss, "si1", "we1", secret+4, set([0,1,2]), 100)
         shares2 = self.allocate(ss, "si1", "we1", secret+5, set([0,1,2]), 100)
+        # cancel one of them
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret+5))
 
         # and write enough data to expand the container, forcing the server
         # to move the leases
@@ -714,3 +725,21 @@ class MutableServer(unittest.TestCase):
 
         # TODO: read back the leases, make sure they're still intact. We need
         # a renew_lease() call for this.
+        ss.remote_renew_lease("si1", self.renew_secret(secret))
+        ss.remote_renew_lease("si1", self.renew_secret(secret+1))
+        ss.remote_renew_lease("si1", self.renew_secret(secret+2))
+        ss.remote_renew_lease("si1", self.renew_secret(secret+3))
+        ss.remote_renew_lease("si1", self.renew_secret(secret+4))
+
+        # now cancel them all
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret))
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret+1))
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret+2))
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret+3))
+        # slot should still be there
+        shares3 = ss.remote_get_mutable_slot("si1")
+        self.failUnlessEqual(len(shares3), 3)
+        ss.remote_cancel_lease("si1", self.cancel_secret(secret+4))
+        # now the slot should be gone
+        self.failUnlessEqual(ss.remote_get_mutable_slot("si1"), {})
+