From 11482da9077889afd701aa1f7c7b0a4a0cebe5db Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone <exarkun@twistedmatrix.com> Date: Thu, 18 Nov 2021 16:24:13 -0500 Subject: [PATCH] add min lease renewal time to the config object as well --- src/_zkapauthorizer/_plugin.py | 57 +---------- src/_zkapauthorizer/config.py | 94 +++++++++++++++++++ src/_zkapauthorizer/lease_maintenance.py | 39 ++++---- src/_zkapauthorizer/tests/strategies.py | 1 + .../tests/test_lease_maintenance.py | 20 +++- 5 files changed, 137 insertions(+), 74 deletions(-) create mode 100644 src/_zkapauthorizer/config.py diff --git a/src/_zkapauthorizer/_plugin.py b/src/_zkapauthorizer/_plugin.py index fd2a669..2c80e2a 100644 --- a/src/_zkapauthorizer/_plugin.py +++ b/src/_zkapauthorizer/_plugin.py @@ -18,7 +18,7 @@ Tahoe-LAFS. """ import random -from datetime import datetime, timedelta +from datetime import datetime from functools import partial from weakref import WeakValueDictionary @@ -27,13 +27,13 @@ from allmydata.client import _Client from allmydata.interfaces import IAnnounceableStorageServer, IFoolscapStoragePlugin from allmydata.node import MissingConfigEntry from challenge_bypass_ristretto import SigningKey -from isodate import parse_duration from twisted.internet.defer import succeed from twisted.logger import Logger from twisted.python.filepath import FilePath from zope.interface import implementer from .api import ZKAPAuthorizerStorageClient, ZKAPAuthorizerStorageServer +from .config import lease_maintenance_from_tahoe_config from .controller import get_redeemer from .lease_maintenance import ( SERVICE_NAME, @@ -230,20 +230,7 @@ def _create_maintenance_service(reactor, node_config, client_node): get_root_nodes=partial(get_root_nodes, client_node, node_config), storage_broker=client_node.get_storage_broker(), secret_holder=client_node._secret_holder, - # The greater the min lease remaining time, the more of each lease - # period is "wasted" by renewing the lease before it has expired. The - # premise of ZKAPAuthorizer's use of leases is that if they expire, - # the storage server is free to reclaim the storage by forgetting - # about the share. However, since we do not know of any - # ZKAPAuthorizer-enabled storage grids which will garbage collect - # shares when leases expire, we have no reason not to use a zero - # duration here - for now. - # - # In the long run, storage servers must run with garbage collection - # enabled. Ideally, before that happens, we will have a system that - # doesn't involve trading of wasted lease time against reliability of - # leases being renewed before the shares are garbage collected. - min_lease_remaining=timedelta(seconds=0), + min_lease_remaining=maint_config.min_lease_remaining, progress=store.start_lease_maintenance, get_now=get_now, ) @@ -256,46 +243,10 @@ def _create_maintenance_service(reactor, node_config, client_node): reactor, last_run_path, random, - interval_mean=maint_config.crawl_interval_mean, - interval_range=maint_config.crawl_interval_range, + lease_maint_config=maint_config, ) -def lease_maintenance_from_tahoe_config(node_config): - # type: (_Config) -> LeaseMaintenanceConfig - """ - Return a ``LeaseMaintenanceConfig`` representing the values from the given - configuration object. - """ - return LeaseMaintenanceConfig( - crawl_interval_mean=_read_duration(node_config, u"lease.crawl-interval.mean"), - crawl_interval_range=_read_duration(node_config, u"lease.crawl-interval.range"), - ) - - -def _read_duration(cfg, option): - """ - Read an ISO8601 "duration" from the ZKAPAuthorizer section of a Tahoe-LAFS - config. - - :param cfg: The Tahoe-LAFS config object to consult. - :param option: The name of the option to read. - - :return: ``None`` if the option is missing, otherwise the parsed duration - as a ``timedelta``. - """ - # type: (_Config, str) -> Optional[timedelta] - section_name = u"storageclient.plugins.privatestorageio-zkapauthz-v1" - value_str = cfg.get_config( - section=section_name, - option=option, - default=None, - ) - if value_str is None: - return None - return parse_duration(value_str) - - def get_root_nodes(client_node, node_config): try: rootcap = node_config.get_private_config(b"rootcap") diff --git a/src/_zkapauthorizer/config.py b/src/_zkapauthorizer/config.py new file mode 100644 index 0000000..f91dc35 --- /dev/null +++ b/src/_zkapauthorizer/config.py @@ -0,0 +1,94 @@ +# Copyright 2019 PrivateStorage.io, LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from datetime import timedelta +from isodate import parse_duration + +from .lease_maintenance import ( + LeaseMaintenanceConfig, +) + +class _EmptyConfig(object): + """ + Weakly pretend to be a Tahoe-LAFS configuration object with no + configuration. + """ + def get_config(self, section, option, default=object(), boolean=False): + return default + +empty_config = _EmptyConfig() + + +def lease_maintenance_from_tahoe_config(node_config): + # type: (_Config) -> LeaseMaintenanceConfig + """ + Return a ``LeaseMaintenanceConfig`` representing the values from the given + configuration object. + """ + return LeaseMaintenanceConfig( + crawl_interval_mean=_read_duration( + node_config, + u"lease.crawl-interval.mean", + timedelta(days=26), + ), + crawl_interval_range=_read_duration( + node_config, + u"lease.crawl-interval.range", + timedelta(days=4), + ), + # The greater the min lease remaining time, the more of each lease + # period is "wasted" by renewing the lease before it has expired. The + # premise of ZKAPAuthorizer's use of leases is that if they expire, + # the storage server is free to reclaim the storage by forgetting + # about the share. However, since we do not know of any + # ZKAPAuthorizer-enabled storage grids which will garbage collect + # shares when leases expire, we have no reason not to use a zero + # duration here - for now. + # + # In the long run, storage servers must run with garbage collection + # enabled. Ideally, before that happens, we will have a system that + # doesn't involve trading of wasted lease time against reliability of + # leases being renewed before the shares are garbage collected. + # + # Also, since this is configuration, you can set it to something else + # if you want. + min_lease_remaining=_read_duration( + node_config, + u"lease.min-time-remaining", + timedelta(days=0), + ), + ) + + +def _read_duration(cfg, option, default): + """ + Read an ISO8601 "duration" from the ZKAPAuthorizer section of a Tahoe-LAFS + config. + + :param cfg: The Tahoe-LAFS config object to consult. + :param option: The name of the option to read. + + :return: ``None`` if the option is missing, otherwise the parsed duration + as a ``timedelta``. + """ + # type: (_Config, str) -> Optional[timedelta] + section_name = u"storageclient.plugins.privatestorageio-zkapauthz-v1" + value_str = cfg.get_config( + section=section_name, + option=option, + default=None, + ) + if value_str is None: + return default + return parse_duration(value_str) diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py index d4f010d..37bd85d 100644 --- a/src/_zkapauthorizer/lease_maintenance.py +++ b/src/_zkapauthorizer/lease_maintenance.py @@ -357,8 +357,7 @@ def lease_maintenance_service( reactor, last_run_path, random, - interval_mean=None, - interval_range=None, + lease_maint_config, ): """ Get an ``IService`` which will maintain leases on ``root_node`` and any @@ -377,19 +376,15 @@ def lease_maintenance_service( :param random: An object like ``random.Random`` which can be used as a source of scheduling delay. - :param timedelta interval_mean: The mean time between lease renewal checks. - - :param timedelta interval_range: The range of the uniform distribution of - lease renewal checks (centered on ``interval_mean``). + :param lease_maint_config: Configuration for the tweakable lease + maintenance parameters. :param maintain_leases: A no-argument callable which performs a round of lease-maintenance. The resulting service calls this periodically. """ - if interval_mean is None: - interval_mean = timedelta(days=26) - if interval_range is None: - interval_range = timedelta(days=4) - halfrange = interval_range / 2 + interval_mean = lease_maint_config.crawl_interval_mean + interval_range = lease_maint_config.crawl_interval_range + halfrange = lease_maint_config.crawl_interval_range / 2 def sample_interval_distribution(): return timedelta( @@ -419,11 +414,8 @@ def lease_maintenance_service( timedelta(0), ) - def get_config(): - return LeaseMaintenanceConfig( - crawl_interval_mean=interval_mean, - crawl_interval_range=interval_range, - ) + def get_lease_maint_config(): + return lease_maint_config return _FuzzyTimerService( SERVICE_NAME, @@ -437,7 +429,7 @@ def lease_maintenance_service( ), initial_interval, sample_interval_distribution, - get_config, + get_lease_maint_config, reactor, ) @@ -446,10 +438,19 @@ def lease_maintenance_service( class LeaseMaintenanceConfig(object): """ Represent the configuration for a lease maintenance service. + + :ivar crawl_interval_mean: The mean time between lease renewal checks. + + :ivar crawl_interval_range: The range of the uniform distribution of lease + renewal checks (centered on ``interval_mean``). + + :ivar min_lease_remaining: The minimum amount of time remaining to allow + on a lease without renewing it. """ crawl_interval_mean = attr.ib() # type: datetime.timedelta crawl_interval_range = attr.ib() # type: datetime.timedelta + min_lease_remaining = attr.ib() # type: datetime.timedelta def lease_maintenance_config_to_dict(lease_maint_config): @@ -461,6 +462,9 @@ def lease_maintenance_config_to_dict(lease_maint_config): "lease.crawl-interval.range": duration_isoformat( lease_maint_config.crawl_interval_range, ), + "lease.min-time-remaining": duration_isoformat( + lease_maint_config.min_lease_remaining, + ), } @@ -469,6 +473,7 @@ def lease_maintenance_config_from_dict(d): return LeaseMaintenanceConfig( crawl_interval_mean=parse_duration(d["lease.crawl-interval.mean"]), crawl_interval_range=parse_duration(d["lease.crawl-interval.range"]), + min_lease_remaining=parse_duration(d["lease.min-time-remaining"]), ) diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index ec952ec..4a61f8c 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -379,6 +379,7 @@ def lease_maintenance_configurations(): LeaseMaintenanceConfig, interval_means(), timedeltas(min_value=timedelta(0)), + timedeltas(min_value=timedelta(0)), ) diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py index a0a9c3a..2aa0740 100644 --- a/src/_zkapauthorizer/tests/test_lease_maintenance.py +++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py @@ -57,7 +57,9 @@ from twisted.python.filepath import FilePath from zope.interface import implementer from ..foolscap import ShareStat +from ..config import lease_maintenance_from_tahoe_config, empty_config from ..lease_maintenance import ( + LeaseMaintenanceConfig, MemoryMaintenanceObserver, NoopMaintenanceObserver, lease_maintenance_config_from_dict, @@ -78,6 +80,7 @@ from .strategies import ( storage_indexes, ) +default_lease_maint_config = lease_maintenance_from_tahoe_config(empty_config) def dummy_maintain_leases(): pass @@ -237,6 +240,7 @@ class LeaseMaintenanceServiceTests(TestCase): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) self.assertThat( service, @@ -265,8 +269,11 @@ class LeaseMaintenanceServiceTests(TestCase): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, - mean, - range_, + LeaseMaintenanceConfig( + mean, + range_, + timedelta(0), + ), ) service.startService() [maintenance_call] = clock.getDelayedCalls() @@ -308,8 +315,11 @@ class LeaseMaintenanceServiceTests(TestCase): clock, last_run_path, random, - mean, - range_, + LeaseMaintenanceConfig( + mean, + range_, + timedelta(0), + ), ) service.startService() [maintenance_call] = clock.getDelayedCalls() @@ -355,6 +365,7 @@ class LeaseMaintenanceServiceTests(TestCase): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) service.startService() self.assertThat( @@ -388,6 +399,7 @@ class LeaseMaintenanceServiceTests(TestCase): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) service.startService() [maintenance_call] = clock.getDelayedCalls() -- GitLab