Merge adding lease with renewing lease #3773

New Issue

itamarst · 2021-08-18T15:13:18Z

itamarst commented

2021-08-18 15:13:18 +00:00

From a client perspective, the goal is just to have a lease, whether it's a new lease or renewing a lease is irrelevant; renewing is essentially an optimization detail on server side.
Cancelling leases is not actually supported.
The way lease renewal secrets are generated is likely to change in the switchover from foolscap to HTTP.
Internally, the server already treats adding a lease as potentially just renewing a lease.

As such, the distinction between adding a lease and renewing a lease is unnecessary (presuming item 4 is actually correct!). We would like to simplify HTTP protocol by removing it, and we need to make sure server can gracefully handle clients switching their renewal secret.

Therefore, we should merge the two into a single operation:

Validate item 4 above, since it's critical requirement for the rest of this.
In Foolscap client, switch to only using add lease code path.
Make sure server creating renews lease in add path.
On server-side, leave new lease Foolscap endpoint in place for backwards compat.
In new GBS HTTP protocol spec, switch to a single end point for lease creation/renewal, and remove all references to lease cancellation secret.
Clients should continue to try to pass in the same renewal secret, the way they do now.

1. From a client perspective, the goal is just to have a lease, whether it's a new lease or renewing a lease is irrelevant; renewing is essentially an optimization detail on server side. 2. Cancelling leases is not actually supported. 3. The way lease renewal secrets are generated is likely to change in the switchover from foolscap to HTTP. 4. Internally, the server _already_ treats adding a lease as potentially just renewing a lease. As such, the distinction between adding a lease and renewing a lease is unnecessary (presuming item 4 is actually correct!). We would like to simplify HTTP protocol by removing it, and we need to make sure server can gracefully handle clients switching their renewal secret. Therefore, we should merge the two into a single operation: 1. Validate item 4 above, since it's critical requirement for the rest of this. 2. In Foolscap client, switch to only using add lease code path. 3. Make sure server creating renews lease in add path. 4. On server-side, leave new lease Foolscap endpoint in place for backwards compat. 5. In new GBS HTTP protocol spec, switch to a single end point for lease creation/renewal, and remove all references to lease cancellation secret. 6. Clients should continue to try to pass in the same renewal secret, the way they do now.

itamarst added the

labels 2021-08-18 15:13:18 +00:00

itamarst added this to the HTTP Storage Protocol milestone 2021-08-18 15:13:18 +00:00

itamarst self-assigned this 2021-08-18 15:13:18 +00:00

exarkun commented

2021-08-18 15:20:51 +00:00

Regarding item 4 from the first list:

Here is the implementation of add_lease from allmydata/storage/server.py:

    def remote_add_lease(self, storage_index, renew_secret, cancel_secret,
                         owner_num=1):
        start = time.time()
        self.count("add-lease")
        new_expire_time = time.time() + 31*24*60*60
        lease_info = LeaseInfo(owner_num,
                               renew_secret, cancel_secret,
                               new_expire_time, self.my_nodeid)
        for sf in self._iter_share_files(storage_index):
            sf.add_or_renew_lease(lease_info)
        self.add_latency("add-lease", time.time() - start)
        return None

And here is the implementation of MutableShareFile.add_or_renew_lease from `allmydata/storage/mutable.py:

    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)
        except IndexError:
            self.add_lease(lease_info)

And here is the implementation of ShareFile.add_or_renew_lease from `allmydata/storage/immutable.py:

    def add_or_renew_lease(self, lease_info):
        try:
            self.renew_lease(lease_info.renew_secret,
                             lease_info.expiration_time)
        except IndexError:
            self.add_lease(lease_info)

(woops nice divergence in safety properties there)

Regarding item 4 from the first list: Here is the implementation of `add_lease` from `allmydata/storage/server.py`: ``` def remote_add_lease(self, storage_index, renew_secret, cancel_secret, owner_num=1): start = time.time() self.count("add-lease") new_expire_time = time.time() + 31*24*60*60 lease_info = LeaseInfo(owner_num, renew_secret, cancel_secret, new_expire_time, self.my_nodeid) for sf in self._iter_share_files(storage_index): sf.add_or_renew_lease(lease_info) self.add_latency("add-lease", time.time() - start) return None ``` And here is the implementation of `MutableShareFile.add_or_renew_lease` from `allmydata/storage/mutable.py: ``` 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) except IndexError: self.add_lease(lease_info) ``` And here is the implementation of `ShareFile.add_or_renew_lease` from `allmydata/storage/immutable.py: ``` def add_or_renew_lease(self, lease_info): try: self.renew_lease(lease_info.renew_secret, lease_info.expiration_time) except IndexError: self.add_lease(lease_info) ``` (woops nice divergence in safety properties there)

itamarst changed title from ~~Merge adding lease with renewing lease (for immutables)~~ to Merge adding lease with renewing lease

2021-08-18 15:27:15 +00:00

GitHub <noreply@github.com> commented

2021-09-01 14:44:46 +00:00

In 056ee58/trunk:

Merge pull request #1110 from tahoe-lafs/3773.just-add-lease

Get rid of renew_lease client code, in order to simplify the protocol

Fixes ticket:3773

In [056ee58/trunk](/tahoe-lafs/trac-2024-07-25/commit/056ee58e9102abb77b75900fe363872fe328f064): ``` Merge pull request #1110 from tahoe-lafs/3773.just-add-lease Get rid of renew_lease client code, in order to simplify the protocol Fixes ticket:3773 ```

tahoe-lafs added the

fixed

label 2021-09-01 14:44:46 +00:00

GitHub <noreply@github.com> closed this issue

2021-09-01 14:44:46 +00:00

Sign in to join this conversation.