storage: make add-lease work, change default ownernum=1 since 0 is reserved to mean...

diff --git a/src/allmydata/storage.py b/src/allmydata/storage.py
index d5859c8f13f09698068c78944ba0d17e0355bbcc..e2fbb4f82a2d6eacdd65ee6c95d53a2fc7623429 100644 (file)
--- a/src/allmydata/storage.py
+++ b/src/allmydata/storage.py
@@ -615,6 +615,7 @@ class MutableShareFile:
         return leases
 
     def add_lease(self, lease_info):
+        precondition(lease_info.owner_num != 0) # 0 means "no lease here"
         f = open(self.home, 'rb+')
         num_lease_slots = self._get_num_lease_slots(f)
         empty_slot = self._get_first_empty_lease_slot(f)
@@ -649,6 +650,7 @@ class MutableShareFile:
         raise IndexError(msg)
 
     def add_or_renew_lease(self, lease_info):
+        precondition(lease_info.owner_num != 0) # 0 means "no lease here"
         try:
             self.renew_lease(lease_info.renew_secret,
                              lease_info.expiration_time)
@@ -829,11 +831,12 @@ class StorageServer(service.MultiService, Referenceable):
                           "write": [],
                           "close": [],
                           "read": [],
-                          "renew": [],
-                          "cancel": [],
                           "get": [],
                           "writev": [], # mutable
                           "readv": [],
+                          "add-lease": [], # both
+                          "renew": [],
+                          "cancel": [],
                           }
 
     def count(self, name, delta=1):
@@ -1064,7 +1067,7 @@ class StorageServer(service.MultiService, Referenceable):
             yield sf
 
     def remote_add_lease(self, storage_index, renew_secret, cancel_secret,
-                         owner_num=0):
+                         owner_num=1):
         start = time.time()
         self.count("add-lease")
         new_expire_time = time.time() + 31*24*60*60

diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py
index a3b18af52fbe8d41273ee91c20a20a2d62255bc5..96d1c4103506f918eb12f462b545992cf9546bd8 100644 (file)
--- a/src/allmydata/test/test_storage.py
+++ b/src/allmydata/test/test_storage.py
@@ -494,6 +494,17 @@ class Server(unittest.TestCase):
         self.failUnlessEqual(len(leases), 2)
         self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs1, rs2]))
 
+        # and a third lease, using add-lease
+        rs2a,cs2a = (hashutil.tagged_hash("blah", "%d" % self._lease_secret.next()),
+                     hashutil.tagged_hash("blah", "%d" % self._lease_secret.next()))
+        ss.remote_add_lease("si1", rs2a, cs2a)
+        leases = list(ss.get_leases("si1"))
+        self.failUnlessEqual(len(leases), 3)
+        self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs1, rs2, rs2a]))
+
+        # add-lease on a missing storage index is an error
+        self.failUnlessRaises(IndexError, ss.remote_add_lease, "si18", "", "")
+
         # check that si0 is readable
         readers = ss.remote_get_buckets("si0")
         self.failUnlessEqual(len(readers), 5)
@@ -519,7 +530,7 @@ class Server(unittest.TestCase):
         self.failUnlessRaises(IndexError, ss.remote_renew_lease, "si0", rs0)
 
 
-        # cancel the first lease on si1, leaving the second in place
+        # cancel the first lease on si1, leaving the second and third in place
         ss.remote_cancel_lease("si1", cs1)
         readers = ss.remote_get_buckets("si1")
         self.failUnlessEqual(len(readers), 5)
@@ -527,16 +538,18 @@ class Server(unittest.TestCase):
         self.failUnlessRaises(IndexError, ss.remote_renew_lease, "si1", rs1)
 
         leases = list(ss.get_leases("si1"))
-        self.failUnlessEqual(len(leases), 1)
-        self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs2]))
+        self.failUnlessEqual(len(leases), 2)
+        self.failUnlessEqual(set([l.renew_secret for l in leases]), set([rs2, rs2a]))
 
         ss.remote_renew_lease("si1", rs2)
-        # cancelling the second should make it go away
+        # cancelling the second and third should make it go away
         ss.remote_cancel_lease("si1", cs2)
+        ss.remote_cancel_lease("si1", cs2a)
         readers = ss.remote_get_buckets("si1")
         self.failUnlessEqual(len(readers), 0)
         self.failUnlessRaises(IndexError, ss.remote_renew_lease, "si1", rs1)
         self.failUnlessRaises(IndexError, ss.remote_renew_lease, "si1", rs2)
+        self.failUnlessRaises(IndexError, ss.remote_renew_lease, "si1", rs2a)
 
         leases = list(ss.get_leases("si1"))
         self.failUnlessEqual(len(leases), 0)
@@ -1046,28 +1059,39 @@ class MutableServer(unittest.TestCase):
         f.write("you ought to be ignoring me\n")
         f.close()
 
+        s0 = MutableShareFile(os.path.join(bucket_dir, "0"))
+        self.failUnlessEqual(len(s0.debug_get_leases()), 1)
+
+        # add-lease on a missing storage index is an error
+        self.failUnlessRaises(IndexError, ss.remote_add_lease, "si18", "", "")
+
         # re-allocate the slots and use the same secrets, that should update
         # the lease
         write("si1", secrets(0), {0: ([], [(0,data)], None)}, [])
+        self.failUnlessEqual(len(s0.debug_get_leases()), 1)
 
         # renew it directly
         ss.remote_renew_lease("si1", secrets(0)[1])
+        self.failUnlessEqual(len(s0.debug_get_leases()), 1)
 
         # now allocate them with a bunch of different secrets, to trigger the
-        # extended lease code
+        # extended lease code. Use add_lease for one of them.
         write("si1", secrets(1), {0: ([], [(0,data)], None)}, [])
-        write("si1", secrets(2), {0: ([], [(0,data)], None)}, [])
+        self.failUnlessEqual(len(s0.debug_get_leases()), 2)
+        secrets2 = secrets(2)
+        ss.remote_add_lease("si1", secrets2[1], secrets2[2])
+        self.failUnlessEqual(len(s0.debug_get_leases()), 3)
         write("si1", secrets(3), {0: ([], [(0,data)], None)}, [])
         write("si1", secrets(4), {0: ([], [(0,data)], None)}, [])
         write("si1", secrets(5), {0: ([], [(0,data)], None)}, [])
 
+        self.failUnlessEqual(len(s0.debug_get_leases()), 6)
+
         # cancel one of them
         ss.remote_cancel_lease("si1", secrets(5)[2])
+        self.failUnlessEqual(len(s0.debug_get_leases()), 5)
 
-        s0 = MutableShareFile(os.path.join(bucket_dir, "0"))
         all_leases = s0.debug_get_leases()
-        self.failUnlessEqual(len(all_leases), 5)
-
         # and write enough data to expand the container, forcing the server
         # to move the leases
         write("si1", secrets(0),